Beam broken segment: wrong position

• Nov 23, 2012 - 13:12
Type
Functional
Severity
S4 - Minor
Status
closed
Project

Context: commit 8e8ddd7bb (22-11-12)

Step: load or re-created the attached sample score

Result: the broken beam for the first 16th is on the wrong side:

test

Notes:
This is related to, but possibly more general than, issues #14869: [trunk] beam on wrong side for semi-quavers in 6/8 (or similar) time and #15779: 32nd note flag points wrong way in sub-beam. Both were solved at some moment but at least the second is back.

Werner noted once that there are occasions when either beam direction might be needed according to the intended rhythm and a way should be provided to obtain either on request. This would be great, but in the meantime, the most common case should give the most commonly 'correct' result.

Analysis:
The code involved is in file libmscore/beam.cpp, function Beam::layout2(), lines 1780 and following:
   if (a.reduced().denominator() < b.reduced().denominator())
     len = -len;

It attempts to compare the durations of the chord with the broken segment, of the previous chord and of the following chord.

This approach fails to distinguish between the groups marked 1 and 2 in the image above: they have the same duration pattern but require different positions for the broken beam.

A better approach could to determine the position of the beam group in the measure by replacing the above lines with (all steps shown in full; code can be optimized):
   Measure* meas = cr1->measure();
   int measTick = meas->tick();
   ChordRest* cr1 = crl[c1-1];
   ChordRest* cr2 = crl[c1];
   int tick1 = cr1->tick() - measTick;
   int tick2 = cr2->tick() - measTick;
   int ticks = ab.ticks();
   int mod1 = tick1 % ticks;
   int mod2 = tick2 % ticks;
   if (mod1 < mod2)
     len = -len;

if the previous chord is on a 'more important' division of the measure than the target chord, the beam is turned to the left. The example above becomes:

test2

If the approach seems correct, I can provide a pull request with a more compact code.

Thanks,

M.

Attachment Size
test_beam_1.png 7.15 KB
test_beam_2.png 4.29 KB
test_beam.mscx 9.27 KB

Comments

Good catch! It seems exactly the same.

The code I mention is there since at least June (I checked the history of the file in Github) and I was in fact wondering how nobody else noticed it since then (I didn't myself, but I worked mostly on Renaissance music in the last months, where semiquavers are rare and that rhythm even rarer).

Someone did notice it indeed and approximately in the time frame of the code change. This confirms my analysis.

I would gladly mark this issue as duplicate, but this would bury the analysis and patching strategy outlined above, while I would like it to be looked at by some of the core developers before posting a pull request.

Thanks!

M.

Status (old) patch (code needs review) patch (ready to commit)

The above pul request has been merged, thanks to the devs.

Unfortunately, I was at the same time investigating the issue more deeply and found some cases where the above patch fails.

For instance, the '*'s in the below screen shot mark broken beams which are still pointing the wrong way:

NOT FOUND: 1

So, I posted a new, simpler and more correct, patch which gives the following result:

NOT FOUND: 2

Even this second patch still leaves some cases unresolved, with highly subdivided tuplets (10-plet or more I think) and sub-tuplets; however I think we are down to rather corner cases.

Anyway, I'm also working on a way to manually mirror the automatically chosen direction for dubious, special (or plainly wrong!) cases; see #20177: Mirror broken beam direction.

Thanks,

M.

Attachment Size
test_beaming_before.png 11.9 KB
test_beaming_after.png 10.58 KB