Hairpins shifting out of position on systems containing ties from stem
2.1-dev (ad8ef22) / Win 7
The issue was marked as fixed recently: #109021: Hairpin and other symbols are shifting when relayout occurs. And also reported, for 2.0.3, at #142156: Hairpins have shifted out of position.
However it still seems to be an issue in the attached score in 2.1-dev. I have identified two cases where it occurs:
1. There is no line break at the end of a system (haven't tested for page/section breaks).
2. There is a line break but also a courtesy key sig. or time sig. at the end of a system.
If you check the score you'll find that all the hairpins where "2" applies have horizontal offsets that weren't there when the score was closed. Hairpins in other systems are unaffected as long as there is a line break at the end of the system.
Attachment | Size |
---|---|
shifting_lines.mscz | 33.13 KB |
Comments
Is this problem reproducible? If so, can you please provide precise steps to reproduce the problem? Here is what I tried:
1) load score
2) click first note, press up then down
3) save under a new name
4) load original score again
I am now examining both scores side by side looking for discrepancies and not seeing any. All hairpins seem to be exactly where they were before saving.
Hmm, no I take it back, a very slight discrepancy in the one in measure 26. Is that what you mean?
The other question I have is whether 2.0.3 shows this same (or other bad) behavior.
I can answer the latter question now after installing the portable 2.0.3 - yes, it's the same in 2.0.3. So at least this is not a regression. Still not understanding case #1 mentioned in the original report, however. Would like a sample score and steps to reproduce that one.
For #2, the score posted definitely exhibits the problem. Can't say yet if it literally applies to all scores with courtesy key signatures and line breaks. But I will observe the discrepancy is 0.06, which might not even be noticeable at first, but it accumulates each time you save / reload. So over time, the error gets worse.
Wow. After a lot of trial and error, I have managed to narrow this down to the actual cause and reproduce the problem in a score from scratch.
It turns out to be completely unrelated to barlines, line breaks, or courtesy signatures. It's about ties, specifically, ties that begin on upstem notes and for whatever reason are also up (eg, ties in a multivoice context, ties forced up, tie from upstem to downstem note which normally defaults to up). In the original score posted above, it's the tie in measure 26 that triggers the issue. Removing it makes the problem go away.
Here is a much simpler example to demonstrate.
1) load score
2) click first note, up / down
3) save
4) check horizontal offset of hairpin in Inspector
4) close
5) reload
6) check horizontal offset of hairpin in Inspector
Result: hairpin has drifted
It looks like we are widening the measure slightly to accommodate the tie even in cases (like this one) where it isn't necessary. Could be something going awry in the code I added long ago to prevent too-short ties, could be something else, but you can see if you delete the tie in the attached example, the measure gets shorter, for not any good reason. Somehow this discrepancy is also affecting hairpin layout or calculation of position during save or reload. Because if you remove the tie, or change the conditions so it is no longer an upwards tie coming off an upstem note, the problems go away. No unnecessary measure widening, no drifting hairpin.
Reproducible in 2.0.3 so not a regression. If it is about the code I added to prevent too-short ties, then it's been there since well before 2.0 beta 1. Even if is some other aspect of tie layout, chances are it's been there a long time.
So, I notice a behaviour change on November 16, 2015.
- With this nightly, nothing particular : d6ab027
- With this one, the symptoms described above appear: 23c2b54
There is seven commits this day, some seem harmless ("fix compilation" and so on).
You have to note a discrepancy between Page View and Continuous View.
- By doing these steps (with the attached test file in comment #6):
1) in Page View: reset the horizontal offset of the hairpin (no issue in this case)
2) New setting to 1.00 sp
3) Save/ Reload
Result: you receive a gap of 8 ( 0,92 vs. 1.00)
- By applying the same steps in Continuous View, you get 0,75 instead of 1.00.
Don't know, butmaybe related to this commit? https://github.com/musescore/MuseScore/commit/149ea937a17b7f743d93c9ba7…
Or, the previous one? https://github.com/musescore/MuseScore/commit/d8e437343562c47561d4a14ca…
Is the horizontal drift of hairpins in measure 8–10 the same issue (as measure 26)? To reproduce, set the horizontal offsets of all hairpins in measures 8–10 to zero, then save and reload the file.
The Vertical offset setting also seems to be a factor. For example:1. Apply a guitar barre line (from Advanced workspace > Lines palette) to the third chord in measure 24.2. Save and reopen.
Result: No change (as expected).
3. Now change the vertical offset of the barre, save and reopen.
Result: There is a very small change in the horizontal offset.
Crossed out non-relevant info. See #11.
Huh... one by one please... I already spent a few hours on it.
cadiz1 - very interesting! I can't see offhand what about either of those commits might be relevant, but I'll check further, also regarding the other commits that day. Thanks as always for your investigation
As I said before, the drift in measure 26 was the only one I saw originally, because I was looking for courtesy key signatures. But yes, measures 8-10 have the same issue - there are three such ties one that system. Remove them and the problem goes away. Again, it was this was in 2.0.3 as well, so not a regression at least.
The vertical change is not especially relevant really. The problem affects any line with a manual adjustment of any kind - vertical or horizontal. Lines with no adjustment have no problem, lines with any manual adjustment have that adjustment calculated incorrectly on load, somehow connected with the tie, although it still isn't clear yet what the connection is.
Hmm, cadiz1, are you sure you got the date right? Now that we have access to the archives of nightly builds (thanks, lasconic!) I went and download the build you say worked, but it shows the exact same result for me. As does a built from earlier in the year - July 1. But a build from June 1 works. I don't have all the builds already downloaded so I have to download them one at a time to test, and I'm between classes right now and can't investigate further until later, but it sure looks to me that it actually broke some time in May 2015.
I created a new version of the test file attached below, since the version I originally created in 2.1 yields an error when loaded into builds this old.
Actually, still looking, might be able to narrow it down to the date before I need to leave...
Got it. It's May 6, 2015. Last build of May 5 works, but first/only build of May 6 fails. Unfortunately, lots of commits around that time, not sure how to sort it out yet. But I'm inclined to suspect it is related to this:
https://github.com/musescore/MuseScore/commit/61eed114019e58e5206ab5bd4…
There is a nightly build with that commit that works, but the build supposedly failed and was fixed later, so I am still suspicious, since this is the only commit around that time that obviously touched the relevant code. It was supposed to be just cleanup, no logic changes, but perhaps a typo introduced a problem. Now I need to go :-)
Yes, absolutely, I am sure. Your new file does not change anything. I loaded it (new, every time)
- with the 2.1 R.C: the horizontal offset of the hairpin is 0.48. After your steps (A -> A # -> A -> Ctrl + S -> Close / Reload), I receive a new offset of 0.39.
- with the nightly of November 16, I get strictly the same thing (0.48 vs. 0.39)
- with the nightly of November 10, I have 0.47 after load, and after steps, I get the same 0.47 after reload.
That being so, it is a date. It is possible, as I have seen several times, that this question has several ramifications.
Very strange indeed! Do you also see any change between the two versions I mentioned? Or in general, between April and June? In particular, does that May 6 build work for you?
You are testing on Windows, or some other OS?
All tests on Windows 7 or 10, as always.
BTW, a typo in an earlier post suggested Hune 1 worked for me. It doesn't. It really is the case that for me, every build I try before May 6 works, every build from then on doesn't. Still scratching my head over how it could be different for someone else using the same builds, also on Windows, but we'll figure it out I'm sure!
One possibility: If the DPI change in November really is relevant, then I guess it could relate to screen resolution. There are various places where we calculate sizes and positions to account for screen resolution, and if we are getting the calculation wrong, I could see it working for one screen resolution but not for another.
Also, it's possible the bug manifests in part on read and also in part on write, meaning in order to really do a good test, you should actually repeat those steps *twice*. Or, do what I actually ended up doing: doing a reset of the horizontal position to be get a clean slate, then explicitly setting it to 0.5, then doing the up/down, *then* finally saving and reloading. Again, works May 5 (and earlier), fails May 6 (and later).
"but first/only build of May 6 fails. "
"first/only" ?
There is 4 nigthlies on May 6, 2015.
The first one: 499ea8e
Another one ( 5c862c3)
the third: a7db24c
and the fourth: 0bd0092
And here (Windows7)
with 151c717 (May 5, last one), it fails,
and with this one 499ea8e (May 6), it works.
And so, here, broken (or re-broken?) on 16 November 2015.
Not sure how I missed those other May 6 nightlies. But for me, all May 5 nightlies work fine, but starting with the first May 6 nightly it fails, and never starts working again. At least, I see the same error May 9, May 12, May 19, June 1, July 1, and November 10, as well as 2.0.3 and the current build. And it works fine January 1, February 2, April 1, and May 1. So for me it does not seem to have ever started working again. Very very very strange!
Still thinking that maybe the screen resolution will turn out to be relevant - somehow we aren't scaling according to "spatium" somewhere that we should be - or maybe are scaling twice.
BTW, the problem also goes away if you go to Style / General / Slurs/Ties and set the "Minimum tie length" to 0. So in case there was any doubt, it does relate to the code that ensures a minimum tie length.
No sure the ties are the only culprit.
Eg (with 2.1 R.C.)
1) "Save selection" of the measure 26 in original file.
Observe the behavior of the hairpin with the tie: select1.mscz
and without (ie, same result) : select2.mscz
2) In original file, you notice the scaling staff space (in Layout /Pages settings) is 1,940mm.
Now, change this scaling to 1.964 mm (the second default/"automatic" value after 1.764mm)
Repeat the same steps (first A up/down, or with the E tied), Save/Close/Reload)
Result: the horizontal offset of the hairpin (with a setting eg of 1.0sp) doesn't change, despite the presence of the tie, and despite other loads/same steps/reloads.
Regarding #1 - that may indeed be an important clue. The tie is definitely changing the width of the measure (delete it and the measure shrinks slightly) but for whatever reason, this is not causing a problem in the hairpin position on save/reload. So it probably isn't the tie itself, or even the effect it has on that measure alone, but something else beyond that.
I can speculate: it looks like the problem occurs only if the extra space caused by tie affects the start position of the measure containing the hairpin. This won't ever happen for hairpins in the first measure of a system of course - they always start in the same place regardless of their contents. Maybe that's not exactly it, but probably something along those lines.
As for #2, again, you and I apparently see different things, because for me it fails regardless of the staff space setting. However, the fact that you see different results depending on staff space setting does lend some support for my theory that the issue is connected to the DPI calculations,. because the DPI calculation is involved in how the "sp" value is stored internally.
What I am about to say gets a bit more technical, but it's kind of thinking aloud for me because I don't want to forget this, and maybe it triggers something for someone else.
Little-known fact - internally, we don't store the "sp" value as millimeters, or inches. Instead they are stored in a unit that, until the big DPI change, depended on screen resolution. And that meant that internally, the same "sp" value (like 1.764mm) would be represented differently on a system with a 100 DPI screen from one with a 200 DPI screen (which is what mine is). The DPI change basically caused us to always represent things internally as if DPI was 72, regardless of what the actual DPI is. We then scale everything up or down according to the actual screen resolution when we need to. That's the general idea anyhow, I might be off in the details.
As I recall, before the DPI change, the actual internal value for the default "sp" would vary from around 5 to over 10; I seem to recall it being 8.333 on my previous system. Now, however, I believe it will always be right around 5 (could be off slightly do the rounding during conversions). That is, regardless of your screen resolution, the internal variable "_spatium" will always report a value of very close to 5 if the score itself uses the default 1.764mm. When we calculate sizes and positions, we do so in these units, and then scale as necessary according to screen resolution.
I have a hunch we are in some place or other not doing this scaling calculation correctly. We are trying to use the "_spatium" in a place that needs to have the DPI scaling performed but isn't - or we are scaling the "_spatium" value somewhere that we shouldn't. So it happens to work out for some screen resolutions and some "sp" values values, but not others.
This is my hypothesis based on the evidence so far - the fact that cadiz1 sees a difference in behavior the day of the big DPI change and also based on the "sp" value for a score, the fact that I see a difference in behavior the day we made a change in what we called some variables and functions that could have easily contained a typo that caused us to use the wrong version of one of these at some point, the fact that was also another change around that point involving ties that was supposed to only affect tablature but might also have accidentally affected pitched staves, and the fact that when I look at the code now, I am scratching my head trying to figure out if we are using "_spatium" correctly or not.
That's all the time I have today, but I will try to look more this weekend.
So, a new test file from scratch, with cases as complete as possible I hope, about this issue.
Here: test file.mscz
(2.1 R.C / Windows 10)
As you can see, the configuration is:
- 3 measures
- measure 2 filled with quarter notes and a tie
- another voice (II, whole rest)
- hairpin in same measure with an horizontal offset of 1.00sp
- scaling staff space (in Page Settings) of 1.900mm
With this configuration, and steps:
1) Load the file
2) Select first note -> Up -> Down
3) Save/Close
4) Reload
Result: the hairpin has a "new" horizontal offset of 1.05, and grows after with same steps, to 1.11, then 1.16 etc.
- The problem is solved (with always the same original test file, and same steps ; in your tests for each case, to be sure, don’t forget to first reset the offset, then new setting to 1.00 -> select first note etc. ):
1) by deleting the voice 2 (whole rest)
2) by keeping the voice 2, but after flipping the stem (X) of the first note E, then pitch Up/Down etc.
3) by deleting the tie
4) by changing the scaling staff space to 1.964mm
5) by deleting (Ctrl + Del) the first empty measure /or the first and third empty measures.
Note: by deleting only the third measure, I get an horizontal offset of 1.08
Observe also that I cannot reproduce with the same file with a Scaling staff space of 1.700 and 1.800 (and 2.000). By returning to 1.900, the issue re-appears.
Thanks as always for the analysis! I hope to have time over the next few days to get to the bottom of this. Armed with this much information one would think it would be easy - and I'm guessing it will turn out to be a one-line fix involving how we account for the "sp" value when ensuring the minimum tie length - but it's still a bit mystifying to me.
Another clue: the offset for the hairpin changes the moment you change the "sp" value (no save/load required). This isn't true for any other element type, nor should it be.
I know the code I wrote to ensure the minimum tie length works in two stages, where we calculate a start position for the tie at one place, then basically repeat the same calculation later to figure out if we need to allocate more space. I made a note in the code that those calculations have to be kept in sync with each other, and had a feeling when I wrote it I was asking for trouble, but I couldn't see another way. That was, as far as I can recall, my very first foray into improving the layout code, and I'll bet if I had it to do over again I'd have been able to figure out something better. But I don't want to rewrite the code right now, I just want to find the discrepancy and fix it :-)
Wait a minute, here's something else odd, since different people seem to see different things, I'd like someone else to confirm this:
Above, I said the hairpin changes position immediately upon a change in "sp". For me, if I have the hairpin at a horizontal offset of 1.00sp, then change "sp" from 1.9mm 2.9mm, the hairpin offset changes to 0.66sp (tested with my own build of the current 2.1 code).
However, the exact same thing happens if I remove the tie first. So that much at least appears to be a bug in the hairpin layout code (well, it applies to other lines as well) that has nothing to do with the tie issue.
Can anyone else check and see if this is true for them?
" if I have the hairpin at a horizontal offset of 1.00sp, then change "sp" from 1.9mm 2.9mm, the hairpin offset changes to 0.66sp"
Confirmed for me.
"However, the exact same thing happens if I remove the tie first."
Confirmed, too.
Thanks for the confirmation!
It turns out the drift that occurs on spatium change might not be relevant. It is a very simple fix but it has no effect on the original problem. FWIW, the drift-on-spatium-change problem also affects arpeggios, stems, and time signatures. These are the element types that override spatiumChanged() but do not then call Element::spatiumChanged(), which is needed to update the user offset. It also affects beams in a way, but these are handled rather differently, and I'm less clear on how those numbers actually work.
In answer to #5. Don't worry about case "1"—it's wrong. And case "2" is only half right. The common factor seems to be a courtesy element at the end of a system—line break or no line beak.
If you have an example of a hairpin shift that is triggered by a courtesy element, I need an example. The example you posted is due 100% to the tie, the courtesy element is irrelevant. As I said, remove the tie, the problem goes away, courtesy or no courtesy.
I might have figured out the problem, not quite what I was thinking but still consistent with the facts. Basically, a rookie mistake of not accounting for floating point round-off - comparing two values that *should* be equal but one is actually ever so slightly bigger than the other. This type of error would indeed be sensitive to spatium and to DPI, because they affect the calculation of the values I am comparing.
I can fix this easily enough, although I still need to remember what is going on in this bit of code a bit better before I decide on the best fix. But - I'm not *totally* sure this is the entire problem, given the fact that different people see different things.
If I'm right, the behavior should also depend on whether the first note of the tie is part of a chord or not. It might also depend on whether there is a second involved. So i hate to be a pest, but since we seem to be seeing different things (to be expected if I'm now right about the cause), could I get some more verification?
In my example, where the first tied note is an "A" on the staff, adding a "C" below that (so the stem is still up) makes the problem go away in my tests. As does adding a "G" which creates a second, with "A" moving to the right of the stem. Adding a "B" changes things differently - the tie flips down - but for me, the problem goes away in this case as well, whether I leave the slur down or flip it back up.
I would love it if someone could test any of this as well! That is, in my test score and steps from #6, any of the following should make the problem go away:
1) adding an "F" below the first "A"
or
2) adding a "G" below the first "A"
or
3) adding a "B" above the first "A" and allowing the tie to flip down
or
4) adding a "B" above the first "A" and flipping the tie up
If anyone finds one of those does *not* make the problem go away, let me know. Realistically, it won't mean I'm not correct about the floating point round-off problem; it will just mean there is *another* problem as well (the code that depends on this floating point comparison is still suspect to me at this point as well).
Here, cases 2, 3, 4 works. The hairpin offset remains to 1.00
But with case 1 (adding a F below the first A), I receive 0.92, instead of the expected 1.00.
Ie this test file with the F: hairpin-shift-tie.mscx
And by continuing (pitch up/down/save/reload), I get 0.83 etc.
Thanks, I think that tells me what I need to know. I should have a fix in the next day or two.
Del.
The code that is triggering the problem is trying to deal with chords with seconds, and is basically messing up even just determining if there *are* seconds involved. And then it may or may not really work as intended in all cases after that, depending on which side of the stem the tied note is on. The following example shows how 2.1 currently deals with one particular situation. There is supposedly a tie between the C's in the following example, but you can just barely even see it:
This isn't a case of the floating point comparison not working correctly - it's that whole section of code handling seconds not really doing what it needs to be doing. So the right fix won't be the one-liner I hoped for. Still, it won't be a big change, and it should fix both aspects of the issue.
https://github.com/musescore/MuseScore/pull/3143
I opened a new report for the issue in #33: #188461: Bad tie lengths on mirrored end notes. My PR above does not address this, except to add comments to help in further investigation.
Fixed in branch 2.1, commit 12b35d26b8
fix #187151: hairpin drift due to bad tie calculation
To be checked in master
Note to self: the code I am dealing with here is in Chord::layoutPitched(), which is setting the left space for a chord element. I was under the impression we are using shapes instead of left/right space now, but I don't see any evidence of this in layoutPitched(), so I'll definitely be curious to see what is going on there.
FWIW, this does not seem to be an issue currently on master, because the code I previously to ensure a minimum tie length no longer has any effect at all. So I need to look into how to make this happen in the new layout scheme, and then make sure not to make the same mistake again.