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 #281831 text cursor init w/special symbols #4588

Conversation

ericfont
Copy link
Contributor

@ericfont ericfont commented Jan 15, 2019

Previously double-clicking text cursor next to a special text such as piano or forte dynamics or the notehead of tempo text would cause musescore to set the cursor to use a special font family ScoreText. This caused problems when typing more text because this special ScoreText font doesn't actually exist in the list of fonts, and so musescore would default to an undesired one (on my arch linux computer would be Bitstream Vera Sans) instead of the text element style's default. This is fixed by having TextCursor::updateCursorFormat() initialize the cursor (to use the default) if the fontFamily was the special 'ScoreText'.

Another problem occured when setting the cursor to be before the initial character when text started with one of these special characters, whereby the fontFamily would be an empty string. This was because TextCursor::set() wouldn't call updateCursorFormat() if the cursor was first placed at row and column 0, due to what looks like a problematic attempt at an optimization. Seems like the optimization was to not perform updates if the text cursor and row indexes (edit: didn't) change from last click...but this optimization failed to consider the case that the TextCursor has row and column initilized to 0, so the updateCursorFormat() wouldn't happen. This is fixed by moving updateCursorFormat() line to be after the if (oldRow != _row || oldColumn != _column) block, so that it always gets called even when first double-clicking the initial character of a text element.

@ericfont
Copy link
Contributor Author

original issue report: https://musescore.org/en/node/281831

@ericfont
Copy link
Contributor Author

Note this seems to fix related issues:
https://musescore.org/en/node/279702 "Make text in dynamics default to italic"
https://musescore.org/en/node/279703 "Using ctrl+a then delete to edit tempo text keeps custom formatting"

Previously, double-clicking text cursor next to a special text such as piano or forte dynamics or the notehead of tempo text would cause MuseScore to set the cursor to use a special font family string claled 'ScoreText'.  This caused problems when typing more text because this special ScoreText font doesn't actually exist in the list of fonts, and so musescore would default to an undesired one (on my arch linux computer would be Bitstream Vera Sans) instead of the text element style's default.  This is fixed by having TextCursor::updateCursorFormat() initialize the cursor (to use the text element style's default) if the fontFamily was the special 'ScoreText'.

Another problem occured where setting the cursor to be before the initial character would result in the fontFamily string being an empty string (again causing confusion from not being able to find an actual font corresponding to empty string).  This was because TextCursor::set() wouldn't call updateCursorFormat() if the cursor was first placed at row and column 0, due to what looks like a problematic attempt at an optimization.  Seems like the optimization was to avoid performing updates if the text cursor and row indexes didn't change position...but this optimization failed to consider the case that double-clicking before the first character corresponds to row 0 and column 0, which is the same values as what TextCursor's constructor ititializes row and column to...that resulted in the updateCursorFormat() line not being executed.  This problem is fixed by moving updateCursorFormat() line to be after the if (oldRow != _row || oldColumn != _column) block, so that it always gets called even when first double-clicking the initial character of a text element.
@ericfont ericfont force-pushed the 281831-text-cursor-default-instead-of-empty-or-ScoreText branch from 970b233 to 560f58f Compare January 15, 2019 20:45
@ericfont
Copy link
Contributor Author

Note, the build passed...but I had some messy wording in the first commit message...so I did a force push with that wording cleaned up.

@anatoly-os
Copy link
Contributor

@ericfont thank you for very descriptive commit message!

@anatoly-os anatoly-os merged commit 782b963 into musescore:master Feb 6, 2019
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