Elements that need fixing after Staff Type Change

• Jul 24, 2020 - 15:41
Reported version
3.4
Type
Graphical (UI)
Frequency
Once
Severity
S3 - Major
Reproducibility
Always
Status
PR created
Regression
No
Workaround
No
Project

Related issues:
- https://musescore.org/en/node/290157
- https://musescore.org/en/node/307905

There is a number of elements that don't follow or don't behave correctly with StaffType properties like: yOffset, stepOffset, number of Lines and Staff Line Distance (mainly but not only noticeable on Staff Type changes). Since the code is related, I'm trying to identify, collect and track tem below:

Ledger Lines: (PRs already created)
- Completely ignore yOffset (fixed on PR #6331 - see https://musescore.org/en/node/290157)
- Misaligned when using arbitrary staff line distances. The ledger line's distance should be the same as the staff line's distance (fixed on PR #6342 - see https://musescore.org/en/node/307904)

Slurs (in process of fixing)
- They completely ignore yOffset (I reported this in https://musescore.org/en/node/307905)

Clefs (fixing)
- There is code to compensate clefs' vertical position after stepOffset, but they move in the wrong direction.
- The code above moves the clef X number of lines above or below, but it should move them <0.5 * stepOffset> , because value considers lines and spaces.
- When changing number of lines, clefs get displaced up or down and end in the wrong staff line in relation to the pitch they mark. (example: if a staff changes from 5-lines to 6-lines, the G clef jumps down one line, where the E is located. It should better ignore the number of lines and remain on the same position unless moves the whole notation up or down).
* Note: when fixing this, consider making percussion clefs ignore so they stay always centered to the staff - since they are not marking any pitch.

Lines (8va, hairpins, text lines, etc)
- Same as slurs, all other lines ignore staff type . They might seem to follow the offset but that's because Automatic Placement is on. With autoPlace off, they don't follow .

Dynamics
- It seems they DO compensate for staff type but they overcompensate (move twice the required distance) when Automatic placement is ON. Needs fixing, and this should be taken into account when fixing Lines above so the same thing doesn't happen.

Staff Text
They seem to follow yOffset correctly but it's good to check on what should happen to their visible anchor points, which stay unchanged (their anchor lines usually go to the top of the staff on the attached segment, but if a staff change yoffset is applied to lower the staff, the anchor pink point on screen points to mid air.)

Grace Notes
- Ignoring yOffset but OK with stepOffset.

Tremolos
- Fingered (between notes) work perfectly. Single note Tremolos (on stem) are ok with yOffset but ignore stepOffset.

Chord diagrams
- Same as Lines, they ignore yOffset but automatic placement works fine. Not a big deal but they should be fixed along the other elements.

Key signatures
- Correctly follow yOffset (accidentals stick to their lines) but don't point to the proper lines if stepOffset is not zero or when number of lines changed.
Obviously it's very unusual to use traditional key signatures with a different number of lines, but if Clefs are to be fixed to point to the right pitch line or space, Key Signatures should be fixed along (so i.e. he F# accidental on an armature always points to the F line).

I also checked Fingering, repeat measures, added barlines, time signatures and they seem to be OK, as well as all System attached elements (obvious but just a sanity check).

Looking at the number of elements that need fixing, maybe an improvement on the way this is handled on a global way might be better thought at some point, but since each of the issues require just very few lines of code or a quick revision, I'm trying to go through them on a PR. Any help and feedback is welcome.


Comments

FWIW, I did a quick experiment a couple of weeks ago in investigating #307659: Drumset edits have no effect after a change instrument object. I did a global search for something like instrument() and replaced everything I could with instrument(tick). I think this in itself would fix a whole bunch of things, and gets us closer to being able to support changing between pitched and unpatched on a single staff. This is instrument, not staff, but I think a similar search would be useful.

I'd be inclined to see a little overhaul of how staff & instrument properties are exposed so that you always need to specify a tick parameter even if it's explicitly -1 (better than 0, btw, because 0 could mean a change right at the start).

You are right @Marc Sabatella, I found "unticked" staffType() queries that lead to some elements I forgot about.

  • Tuplets (need to check)
  • Beams (they have some engraving "rules" about beam's position on the staff that need fixing after Staff type changes' yoffset)
  • Bounding Boxes

In some cases (maybe staffLines.cpp) I will have to check bounding boxes correction too, because the yOffset doesn't adjust the bbox and selecting the element (i.e. selecting a measure) can be difficult.

Note: Actually, several calls for staff()->staffType(tick) should replaced by plain Element::staffType() which already checks for (staff()) and returns what is needed.

const StaffType* Element::staffType() const
{
Staff* s = staff();
return s ? s->staffTypeForElement(this) : nullptr;
}