Skip to content

[MU3 Backend] ENG-59: Fix missing secondary beam breaks #8492

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 5, 2021
Merged

[MU3 Backend] ENG-59: Fix missing secondary beam breaks #8492

merged 1 commit into from
Jul 5, 2021

Conversation

iveshenry18
Copy link
Contributor

Resolves: ENG-59: Fix missing breaks in secondary beams

Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete when beam modes are specified explicitly in the
.mscx; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

@iveshenry18
ENG-59: Fix missing secondary beam breaks
812ae4f 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Jul 2, 2021
Closed
@vpereverzev vpereverzev merged commit ab406e2 into musescore:3.6.2_backend Jul 5, 2021
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 6, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
5b1a8f0 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492
Jojo-Schmitz
Jojo-Schmitz reviewed Jul 6, 2021
View reviewed changes
importexport/musicxml/exportxml.cpp
// when equivalent to BEGIN32 or BEGIN64
if ((blp < i && bln >= i)
|| bmc == Beam::Mode::BEGIN32 && i > 1
|| bmc == Beam::Mode::BEGIN64 && i > 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

this introduces compiler warning, suggetion to add parens or the && conditions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#8554

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Likewise! I've been trying to keep a closer eye on warnings as I compile locally; hopefully no more will slip through!

Jojo-Schmitz
Jojo-Schmitz reviewed Jul 6, 2021
View reviewed changes
importexport/musicxml/exportxml.cpp
else if (blp >= i && bln < i)
else if ((blp >= i && bln < i)
|| bmn == Beam::Mode::BEGIN32 && i > 1
|| bmn == Beam::Mode::BEGIN64 && i > 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 6, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
e628993 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 6, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
3507e7a 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings
@iveshenry18 iveshenry18 mentioned this pull request Jul 6, 2021
Merged
8 tasks
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 6, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
63a7fb1 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 13, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
d737789 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
c118cb2 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
4d143f6 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
90e64f5 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
de893d5 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
399cb03 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
2a2edc6 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 28, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
e556d22 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 28, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
e83e6a4 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 13, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
9105a0f 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 13, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
d6611d9 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 19, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
7656281 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 19, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
4732e48 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 30, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
5abc7f9 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Sep 1, 2021
Closed
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 1, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
287f8c5 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 2, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
b335db3 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 10, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
a8569c4 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 23, 2021
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
ef27659 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Mar 1, 2023
Closed
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 5, 2023
@iveshenry18 @Jojo-Schmitz
ENG-59: Fix missing secondary beam breaks
e1199fd 
Previously, only the first beam was taken into account when importing
Beam::Mode. This commit adds a mechanism to collect the beam type
for all beams and make a more correct choice about Beam::Mode (including
BEGIN32 and BEGIN64). Additionally, it adds handling of these modes to
the export code. Now the round trip from .mscx -> .xml -> .mscx is
correct and complete *when beam modes are specified explicitly in the
.mscx*; however, more thorough handling of Beam::Mode::AUTO (where it
results in the equivalent of BEGIN32 or BEGIN64) on export needs to be
done.

Duplicate of musescore#8492, plus fixing 4 compiler warnings, see musescore#8554
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jojo-Schmitz Jojo-Schmitz Jojo-Schmitz left review comments

@its-not-nice its-not-nice its-not-nice approved these changes

@vpereverzev vpereverzev vpereverzev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iveshenry18 @Jojo-Schmitz @its-not-nice @vpereverzev