Skip to content
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 308242: Elements that need fixing with Staff Type Changes #6376

Merged
merged 1 commit into from
Nov 30, 2020

Conversation

elerouxx
Copy link
Contributor

@elerouxx elerouxx commented Jul 25, 2020

Resolves: https://musescore.org/en/node/308242
https://musescore.org/en/node/290157
and https://musescore.org/en/node/307905

This PR fixes a number of issues with some elements and Staff Type Changes. (Most of the fixes required just adjusting the vertical position to the right staffType yOffset and stepOffset.)

Before:
image
After:
image

Ledger Lines

  • Now generate correctly after top/bottom staff line and stepOffset (chord.cpp)
  • Now follow staffType yOffset (ledgerline.cpp)

Slurs

  • Now follow staffType yOffset

Ties

  • Now follow staffType yOffset
  • Most generated ties (by pasting, changing note duration, etc) didn't have a tick set at creation time, so I went thru all them to set Tick to startNote->tick() (and when possible Tick2 to endNote->tick() ) on tie creation (in cmd.cpp, note.cpp, noteentry.cpp, paste.cpp, edit.cpp).

Before the fix:
image

Clefs

  • Now stay at the right line when using more or less than 5 staff lines.
  • Now are adjusted to mark the right pitch after stepOffset
  • Percussion and tab clefs now ignore stepOffset (since they don't indicate a pitch) so they stay vertically centered to the staff.

Key Signatures

  • Adjusted to follow stepOffset so they stay at right pitches

Repeat barlines

  • Now the dots stay properly centered to the staff with an yOffset

Line

  • Adjusted y pos to follow staffType yOffset (line.cpp)
  • Adjusted anchor lines to reflect staffType yOffset (genericDragAnchorLines in element.cpp)

text line

  • Adjusted to follow yOffset (textlinebase.cpp)
  • Adjusted anchor lines (same as above)

Text

  • Adjusted to follow yOffset
  • Adjusted anchor lines (same as above).

Before the fix:
image

Click note entry

  • Clicking to add notes on a changed staff Type now adds the note in the intended line/space, recalculated after the staffType yOffset, stepOffset and lineDistance. (noteentry.cpp)

Grace Notes

  • Now follow staff Type yOffset

Lyrics

  • Added staff yOffset to Lyrics lines so they stay aligned to lyrics. (lyricsline.cpp)

Compute Up/down:

  • Now the staff middleLine is calculated including stepOffset, so beams and other elements have the right direction.

Additional notes (for a future PR):

  • Symbols and images added to barlines do not to adjust to yOffset (they do when added to chord/rests). Should they?
  • Skylines: check on how rects are added to it on a staff type change with an offset (Maybe don't need a fix - in any case seems too complex to include here)
  • Some articulations like the accent follow stepOffset correctly, but staccato and dash (tenuto) don't. Need further checking on this.
  • Some elements like dynamics seem to 'overdo' automatic placement along with the staffType Offset. Needs deeper checking (not in this PR for the time being).
  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • [NA] I created the test (mtest, vtest, script test) to verify the changes I made

@elerouxx elerouxx force-pushed the elements-fix-staffchange-3.x branch from 155f765 to 2b4175b Compare July 29, 2020 19:11
tie->setTick(note->chord()->segment()->tick());
tie->setTicks(nnote->chord()->segment()->tick() - note->chord()->segment()->tick());
tie->setTick(note->chord()->segment()->tick());
tie->setTicks(nnote->chord()->segment()->tick() - note->chord()->segment()->tick());
Copy link
Contributor Author

@elerouxx elerouxx Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just indented these two lines, but - innocent question: why can't we just tie->setTick(note->tick()) ?

@@ -384,6 +384,8 @@ void Lyrics::layout2(int nAbove)

if (placeBelow()) {
qreal yo = segment()->measure()->system()->staff(staffIdx())->bbox().height();
//TODO: get real height from staffType(tick)- for staff changes (but involves autoplace decisions)
// if (staffType()) { yo = staffType()->lines() * staffType()->lineDistance().val); }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some places on the code that compute positions and autoplacing from the unchanged staff height boundaries. This could be adapted to the real staffType(tick) boundaries but that would be a separate PR.

qreal stYOffset = ss->staffType(p.segment->tick())->yoffset().val();
qreal lineDist = ss->staffType(p.segment->tick())->lineDistance().val();
p.line -= stepOffset + 2 * stYOffset / lineDist;

if (score->inputState().usingNoteEntryMethod(NoteEntryMethod::REPITCH) && !isTablature)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code recalculates the click position on a modified staff so the note is added to the intended (clicked) line or space. (Testing for any other input method that implies clicking or could be affected by this code.)

@elerouxx elerouxx marked this pull request as ready for review July 29, 2020 22:55
@elerouxx elerouxx force-pushed the elements-fix-staffchange-3.x branch from 2b4175b to 9fe3ea6 Compare August 2, 2020 01:47
@elerouxx elerouxx changed the title [WIP] Fix 308242: Elements that need fixing with Staff Type Changes Fix 308242: Elements that need fixing with Staff Type Changes Aug 2, 2020
@elerouxx elerouxx force-pushed the elements-fix-staffchange-3.x branch from 9fe3ea6 to 243f952 Compare August 2, 2020 03:20
@elerouxx elerouxx changed the title Fix 308242: Elements that need fixing with Staff Type Changes [WIP] Fix 308242: Elements that need fixing with Staff Type Changes Aug 17, 2020
@elerouxx elerouxx force-pushed the elements-fix-staffchange-3.x branch 4 times, most recently from 2b0f82d to dae5e11 Compare August 18, 2020 20:57
@elerouxx elerouxx changed the title [WIP] Fix 308242: Elements that need fixing with Staff Type Changes Fix 308242: Elements that need fixing with Staff Type Changes Aug 19, 2020
@elerouxx elerouxx force-pushed the elements-fix-staffchange-3.x branch from dae5e11 to c578328 Compare August 19, 2020 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants