An issue of deciding stem lengths with tremolos

• Jun 22, 2020 - 08:07
Reported version
3.x-dev
Type
Functional
Frequency
Once
Severity
S4 - Minor
Reproducibility
Always
Status
closed
Regression
Yes
Workaround
Yes
Project

As you probably know, 3.5 optimizes the default layout regarding tremolos. However, there's still a tricky issue which I so far cannot diagnose properly. It's about how a stem length should be lengthened when there's a single-note tremolo on it.

The smart algorithm I introduced works well in most cases, because it makes use of a very effective variable: the distance between tremolo strikes and a certain note. This can help calculate the stem lengths so that they look nice most of the time. The problem is, what note should we use, the one furthest to the tremolo strokes, or the one nearest? Intuitively speaking, I would choose the furthest, because a stem ends on that note. But when I tried it, it didn't always work:

if (up())
      height = downPos() - _tremolo->pos().y();
else
      height = _tremolo->pos().y() + _tremolo->height() - upPos();

批注 2020-06-22 144627.png

Apparently, the stems of the beamed chords are lengthened twice the amount needed.

But when I change it to the nearest note, the stem of the unbeamed chord isn't lengthened enough.

if (up())
      height = upPos() - _tremolo->pos().y();
else
      height = _tremolo->pos().y() + _tremolo->height() - downPos();

批注 2020-06-22 145820.png

I tried to understand the main code (Chord::defaultStemLength()) of deciding default stem lengths, and from what I saw, the stem length of a chord does include the distance between notes, which means my first speculation should've been correct. Besides this function, I cannot find another one which decides stem lengths or doubles them when the chords are beamed.

I ended up writing my part of the code like this:

if (up())
      height = (beam() ? upPos() : downPos()) - _tremolo->pos().y();
else
      height = _tremolo->pos().y() + _tremolo->height() - (beam() ? downPos() : upPos());

批注 2020-06-22 150608.png

It seems to work nicely. But I wonder whether there's a more logical solution, as I cannot find clear logic in this one.


Comments

Now that I understand the issue, the proposed solution isn't quite as crazy as it looks.

As I mentioned in the PR comments, the stem length for beamed notes is actually set in beam.cpp. Anything done in minAbsStemLength() is going to be preliminary only for beamed notes. The code in beam.cpp takes that value into consideration when it does its black magic. Probably the real fix is to look at the beamp.cpp code to see more about what it is happening there and if there is a way to avoid that additional lengthening. But it's entirely possible that code is just not very smart and really depends on a more conservative value for minAbsStemLength. I would start by stepping through the lines in beam.cpp where minAbsStemLength is actually used. Offhand, I see some suspicious "+=" statements within a loop, looks like we're growing something each time through, which seems like that can't be right. So maybe that code is just buggy.

Also as I mentioned before, I'm not really a fan of how long the stem stems for the unbeamed note with hook. Gould shows hooks that are more curved to create more room for the tremolo to tuck under, eventually we should consider switching to that design at least in these cases.

In reply to by Marc Sabatella

But so far I haven't seen a precedent for the code-in values of layout depending on the musical font.

And if I have to look into beam.cpp to really fix the issue in a more logical way, it's probably not feasible before 3.5 release. If I touch that part of the code, maybe some new bugs will emerge since many things depend on it. Do you agree if we merge the changes I wrote for now? I can provide some comments in the code for further reference.

Comments would help. At first glance, I saw the dependency on up() and thought it was something totally different going on, and figured it had to do with the need to correct initial guesses about stem direction later in the beam. What you have is pretty conservative, it is just setting a smaller minAbsLength value for beamed notes than other notes (as I now understand it) and relying on the beam code to take it from there.

Even so, do be sure to test a variety of cases - both stem up and stem down, and most importantly, cases where the initial guess turns out wrong. To test that you need to beam two different chords together. Like, in that octave example you have there - I'll assume treble clef and call is two E's - try beaming that to two G's. That will force the stem down. Current 3.x code seems to somehow handle this OK, and it looks like your proposed change preserves the current behavior for beamed notes.

So I think you may be off the hook (hmm, pun not intended) for needing to dig into the beam code.

Fix version
3.5.0