musescore / MuseScore

MuseScore is an open source and free music notation software. For support, contribution, bug reports, visit MuseScore.org. Fork and make pull requests!
https://musescore.org
Other
12.23k stars 2.65k forks source link

Explode often leads to corrupted files #16896

Open Lenbus opened 1 year ago

Lenbus commented 1 year ago

Issue type

Other type of issue

Bug description

Working with multi-staff choir arrangments I often use the explode/implode functionalities. They are quite limited in functionality and could be more advanced but they basically do what they are supposed to do. Unfortunately the Explode feature sometimes results in corrupted files, where e.g. the added notes and full bar pauses exist in parallel in the result measure (both belonging to voice 1).

Steps to reproduce

I encountered this mostly whilst splitting imported MusicXML sheets featureing different voices (polyphnic, also mixed with monophonic passages as well).

Screenshots/Screen recordings

No response

MuseScore Version

4.0.2

Regression

I don't know

Operating system

Windows 11

Additional context

I would really appreciate an advanced version of this feature. E.g. where you can choose the target staffs.

Moreover an option to choose, how to process:

oktophonie commented 1 year ago

It would be very helpful if you could attach it a file, or files, where this can be easily reproduced. (Put them in a .zip first.)

GitWob commented 1 year ago

Hello!

I was having this issue too, and decided to investigate a little. I tested the score that was giving me issues on the latest nightly build.

Bug description Using the Explode tool, then hitting ctrl-Z before doing any other actions, will place notes in another staff, or multiple staves, which corrupt the files.

Steps to reproduce Using my score, but maybe others (I have only tested my own),

Sometimes the explode tool will not act at all. Sometimes it will work and won't make any notes that corrupt the score. In these cases, I don't know if it's another bug, or my selection isn't a valid selection for explode.

Notes that are created by this effect do not go away with undo.

Screenshots image My error message ^ image Before running explode (on the selected notes) and undoing ^ image After, showing the created notes ^

MuseScore Version 2023-04-03 Nightly/Latest

Regression Don't know

Operating System Fedora 37 Workstation

My Score Nonconfrontational (fix copy).zip

Additionally I'm currently unfamiliar with MuseScore's codebase, but I do like to program and would like to try learning about the bug from looking at the source code. Maybe I could even offer a fix. Can anyone point me to where the explode functionality is in the source? Huge thanks!

bkunda commented 1 year ago

Thanks @Lenbus and @UsefulGitHub. I'd probably nominate @cbjeukendrup to see if he can help here, especially with your specific question @UsefulGitHub.

cbjeukendrup commented 1 year ago

@UsefulGitHub The Explode functionality is defined in NotationInteraction::explodeSelectedStaff(), or more specifically in Score::cmdExplode().

GitWob commented 1 year ago

@cbjeukendrup Hi again, I have a better grasp now on what causes the bug.

Explode works as expected when the passage is all in voice 1. In the handbook,

"If the passage contains multiple voices, voice 1 notes are retained on the top staff, while other voices are moved to subsequent staves."

This works as expected too. The voice 2 notes are copied into lower staves using CloneVoice(). The condition for the bug is when the lower staff is only rests. It doesn't matter if it's a measure rest or if the rest is broken up. Whenever it's only rests, the voice 2 notes are copied in as voice 1 notes without getting rid of the voice 1 rests, and the measure has too many notes inside to be musically possible. Here's an example of 9 eighth notes in a 4/4 measure.

image

I've been looking through Score::cmdExplode() specifically for the last few days to see if I can find out why this happens, but I am not understanding the flow of data through the loops, or how the loop control variables work. Maybe this info will help someone else solve the bug, or If anyone is up to helping me read the code, I would appreciate that too.

Thanks again!

cbjeukendrup commented 1 year ago

@UsefulGitHub Thanks for your investigation! I've also experimented a bit, and the problem seems to be that CloneVoice::redo does not take rests into account that start earlier than the first destination segment.

So this goes well:

Scherm­afbeelding 2023-05-07 om 16 15 48

And this not:

Scherm­afbeelding 2023-05-07 om 16 19 24

I think it would be better to use Score::makeGap to clear the destination voice. That method seems a bit more clever and is used in other places too.

I have a feeling that something is not right with undo handling in the CloneVoice class, but let's worry about that later. There's potentially a lot of cleanup to do there, but we should probably handle that internally in the team.