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 #23119: Ensure TextLineBase properties are written for hairpins #23125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mathesoncalum
Copy link
Contributor

Resolves: #23119

Inheritance goes SLine <- TextLineBase <- Hairpin. Hairpins were skipping TextLineBase properties (e.g. gap between text and line).

@cbjeukendrup
Copy link
Contributor

To fix the unit tests, you could of course just update all reference files. But that's tedious and I'm not even sure how desirable the changes are.

Instead, I'd fix it by adding the following somewhere in the Hairpin constructor:

    resetProperty(Pid::END_TEXT_PLACE);
    resetProperty(Pid::BEGIN_HOOK_HEIGHT);
    resetProperty(Pid::END_HOOK_HEIGHT);

Alternatively/additionally, you could investigate what happens when removing the following from the TextLineBase constructor:

    setBeginHookHeight(Spatium(1.9));
    setEndHookHeight(Spatium(1.9));

Because it seems quite weird to set such an arbitrary value for these arbitrary properties in an abstract base class.

The conclusion may be that you need to add these calls to the constructors of some specific subclasses of TextLineBase, but it feels really weird to have that in the base class.

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.

"Gap Between Text and Line" does not survive save and open
4 participants