-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 #15726 (Barlines & Line Breaks): Some elements are lost when changing time signature #26256
base: master
Are you sure you want to change the base?
Conversation
(Updated)This PR try to keep as many barlines as possible. Obviously, the policy can be modified as to which elements and how they should be kept. |
5574453
to
f26e342
Compare
I have created a PR to keep BarLines, Marks and Jumps. Now a test fails, but I think it's related with the new expected behavior. I mean the error is:
This error happens because we know keep BarLines. I mean Original ScoreExpected ScoreScore output with this PRWhy does this happen?We now keep the BarLines, so the output is different because it is now the expected behavior. We can't change it as long as we try to keep BarLines. How should we proceed now? |
1 similar comment
I have created a PR to keep BarLines, Marks and Jumps. Now a test fails, but I think it's related with the new expected behavior. I mean the error is:
This error happens because we know keep BarLines. I mean Original ScoreExpected ScoreScore output with this PRWhy does this happen?We now keep the BarLines, so the output is different because it is now the expected behavior. We can't change it as long as we try to keep BarLines. How should we proceed now? |
eae198b
to
b6e5ecc
Compare
In the unit tests I see lots of |
The first chunk of that diff looks correct to me (as does the start repeat in theScore output with this PR' image), I'd just adjust the test file. |
HI @Jojo-Schmitz . Thanks for your answer
I changed the behaviour to keep as much barlines as possible, keeping them in the middle of a Measure if it was the initial position (Fraction within the score). MuseScore allows that. I have just created from the scratch the following Score: Score_15726_test2ss.zip: I haven't changed the time signature with this PR and you could also see a negative fraction:
Current PR version try always to keep as much barlines as possible when changing TS even if they are in the middle of the Measure. I don't know if this is the expected behavior or not. That's why I have included two scores with duplicated lines ( https://github.com/user-attachments/files/18563141/Score_15726_tests.zip ) to assist in deciding which BarLines should be kept (perhaps, as you suggest only the falling on the final measure boundary, or only the ones which were in the original measure boundary and also in the final measure boundary...). It is up to you. I would appreciate any hint about it |
It it adds/keems/moves a repeat barline mid-measure, it should at least create the appropriate chords/rests before and after, or split the measure at that spot (else the repeat barline won't play) |
Hello again @Jojo-Schmitz . I have seen that this is also an expected behavior with current version (not time signature change involved). I have recorded this video where I get the same behaviour: Issue_17226_Example_middlebar.mp4It seems that the rests isn't splitted ( Score_15726_test4.zip )
Therefore I think there isn't a problem with the PR itself. A remaining question could be if we should keep "middle" (on initial or final scores).. barlines... |
b6e5ecc
to
8fdc09e
Compare
As mentioned in #15726 (comment) I have created a different PR (#26298) for Marks and Jumps and I have updated this PR to include only BarLines |
Hi, I'm confused about whether this is ready to be tested. I've been using 3.7 only, but would be happy to test 4.xx if that would be useful. |
I'm not sure either. This PR try to keep as much BL as possible, but it is my understanding that the creator (@cbjeukendrup) should decide which BL should be kept and the rules. I mean, you can choose between different options:
|
I haven't seen a response from cbjeukendrup, so here's my feedback: Or the last item "keep as much BL as possible and it is to the user to determine which ones should be deleted", I don't understand the scenarios where there is a barline in the middle of a measure; I've never used that or even seen it, so if it were up to me, I would leave it out for now and address specific issues as they arise in the future. |
Thanks for the feedback @BanjoJake. I really appreciate it!. Just to answer and clarify things. Option 3 does not include items 1 & 2. I rewrite these options:
I agree that the third one is the one which could generate less problems. I you agree I will change the PR to agree with the third option.
This is what the PR does now (regardless of potential errors)
I don't really understand either the need for "middle" BL, but I am not a musician expert. You can create any BL wherever you like. Just select a chord or a rest and click on a BL. Example provided: Score_15726_BL_tests.zip. You could use it also as a test Score. Each line is duplicated so you can check the result |
Sorry for my ignorance of Github - where can I find this PR to test ? |
Yes, although I am also happy to test the existing PR if I can find it. |
As usual: the artifacts of the builds, like at https://github.com/musescore/MuseScore/actions/runs/13091403937?pr=26256 (for Windows) |
Thanks, I tested this a while ago (and again just now), and it failed this simple test*, so I didn't think it was what pacebes was referring to. |
I think it's explained at the begining of this Page ( #26256 (comment)). The test fails because It expects the previous behaviour when changing time signature meant BLs were deleted. It now keeps BLs, so this test can't pass. |
I'm sorry, still confused - when you say "test", you're not referring to the test file I just sent ? |
Sorry. My fault. I am reading this from my mobile. and a I missed the file I will test It back in my computer at home |
Hi @BanjoJake. Result file: Test of 'elements lost when changing time signature #15726'_4_4.zip I have tested the file with the version in my computer and also with the version @Jojo-Schmitz suggested: https://github.com/musescore/MuseScore/actions/runs/13091403937?pr=26256 Could you please tell me what's wrong?. InitialFinalI think it keeps all the barlines |
Yes, Thank you, it works ! I don't know where I got the version I was testing ( 4.2.0-233521124/eb8d33c instead of 4.5.0-250321841/2899ec4). I guess I'm not that familiar with the v4 world (I mainly use v3.7). Very sorry for wasting your time and thank you very much again !! |
This is great ! If you're still working on this, I wonder if it would be a big deai to also preserve the line breaks ? That would restore the more even staff spacing in the original. Also wondering when this could be backported to 3.7 ? THANKS !! |
Yes, I can work on it. @Jojo-Schmitz , should I include the preserving of line breaks in this PR?? |
Sure! |
…t when changing time signature
…time signature
8fdc09e
to
1979847
Compare
Done! |
Yes, MuseScore Studio version (64-bit): 4.5.0-250490915 / 9f2c4ae (Windows 10) works perfectly on my test case ! I will try on this some real-world sheets in the next couple of days. Thank you very much !! |
Resolves: #15726 (Barlines && Line Breaks)
(Updated) This PR keeps as much Barlines and Line and page Breaks as possible when changing time signature