Skip to content

Hook height migration #32467

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.

Sign up for GitHub

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

Open
miiizen wants to merge 1 commit into
base: master
Choose a base branch
from
+132 −40
Open

Hook height migration #32467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions src/engraving/compat/engravingcompat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,6 @@ void EngravingCompat::doPostLayoutCompatIfNeeded(MasterScore* score)

int mscVersion = score->mscVersion();

if (mscVersion < 460) {
needRelayout |= resetHookHeightSign(score);
}

if (mscVersion < 440) {
needRelayout |= relayoutUserModifiedCrossStaffBeams(score);
}
Expand Down Expand Up @@ -347,36 +343,4 @@ bool EngravingCompat::relayoutUserModifiedCrossStaffBeams(MasterScore* score)

return found;
}

bool EngravingCompat::resetHookHeightSign(MasterScore* masterScore)
{
bool needRelayout = false;

for (Score* score : masterScore->scoreList()) {
for (auto pair : score->spanner()) {
Spanner* spanner = pair.second;
if (spanner->isTextLineBase()) {
for (SpannerSegment* spannerSeg : spanner->spannerSegments()) {
TextLineBaseSegment* textLineSeg = static_cast<TextLineBaseSegment*>(spannerSeg);
if (textLineSeg->placeBelow()) {
if (!textLineSeg->isStyled(Pid::BEGIN_HOOK_HEIGHT)) {
Spatium beginHookHeight = textLineSeg->getProperty(Pid::BEGIN_HOOK_HEIGHT).value<Spatium>();
textLineSeg->setProperty(Pid::BEGIN_HOOK_HEIGHT, -beginHookHeight);
spanner->triggerLayout();
needRelayout = true;
}
if (!textLineSeg->isStyled(Pid::END_HOOK_HEIGHT)) {
Spatium endHookHeight = textLineSeg->getProperty(Pid::END_HOOK_HEIGHT).value<Spatium>();
textLineSeg->setProperty(Pid::END_HOOK_HEIGHT, -endHookHeight);
spanner->triggerLayout();
needRelayout = true;
}
}
}
}
}
}

return needRelayout;
}
} // namespace mu::engraving::compat
1 change: 0 additions & 1 deletion src/engraving/compat/engravingcompat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,5 @@ class EngravingCompat
static void migrateNoteParens(MasterScore* masterScore);

static bool relayoutUserModifiedCrossStaffBeams(MasterScore* score);
static bool resetHookHeightSign(MasterScore* masterScore);
};
} // namespace mu::engraving::compat
16 changes: 16 additions & 0 deletions src/engraving/rw/compat/compatutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,22 @@ void CompatUtils::setTextLineTextPositionFromAlign(TextLineBase* tl)
}
}

void CompatUtils::resetHookHeightSign(TextLineBase* tl)
{
if (!tl->placeBelow()) {
return;
}

if (!tl->isStyled(Pid::BEGIN_HOOK_HEIGHT)) {
Spatium beginHookHeight = tl->getProperty(Pid::BEGIN_HOOK_HEIGHT).value<Spatium>();
tl->setProperty(Pid::BEGIN_HOOK_HEIGHT, -beginHookHeight);
}
if (!tl->isStyled(Pid::END_HOOK_HEIGHT)) {
Spatium endHookHeight = tl->getProperty(Pid::END_HOOK_HEIGHT).value<Spatium>();
tl->setProperty(Pid::END_HOOK_HEIGHT, -endHookHeight);
}
}

void mu::engraving::compat::CompatUtils::setMusicSymbolSize470(MStyle& style)
{
// Music symbols have their own point size in 4.7
Expand Down
1 change: 1 addition & 0 deletions src/engraving/rw/compat/compatutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class CompatUtils
static const std::map<Sid, Sid> ALIGN_VALS_TO_CONVERT;
static Sid positionStyleFromAlign(Sid align);
static void setTextLineTextPositionFromAlign(TextLineBase* tl);
static void resetHookHeightSign(TextLineBase* tl);
static void setMusicSymbolSize470(MStyle& style);
static Spatium convertPre470FrameRadius(double frameRadius);
static void doMigrateNoteParens(EngravingItem* item);
Expand Down
7 changes: 7 additions & 0 deletions src/engraving/rw/read114/read114.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,7 @@ static void readVolta114(XmlReader& e, ReadContext& ctx, Volta* volta)
}
volta->setOffset(PointF()); // ignore offsets
volta->setAutoplace(true);
CompatUtils::resetHookHeightSign(volta);
Copy link
Contributor

@mike-spa mike-spa Mar 5, 2026

Choose a reason for hiding this comment

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

It makes me a bit uncomfortable to have to add this compat function to 20 different places across all the read module. Could we not move this to EngravingCompat::doPreLayoutCompat? It will perhaps slightly add to the reading time as we need to navigate and find these spanners, but it's well worth it IMO if it saves us from calling it so many times

Copy link
Contributor Author

@miiizen miiizen Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We could, but I'm keen to put as much compatibility code which doesn't require score context in the file reading code as possible. Otherwise we need to add the same compatibility fix for palette items in PaletteCompat, which is something we always forget to do and I'd prefer to avoid where possible. This was part of the motivation behind versioning the palette files.

I agree this is a bit messy though.

Copy link
Contributor

@mike-spa mike-spa Mar 5, 2026

Choose a reason for hiding this comment

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

Ah good point about the palettes, then yeah I think it's fine 👍

}

//---------------------------------------------------------
Expand Down Expand Up @@ -1216,6 +1217,8 @@ static void readOttava114(XmlReader& e, ReadContext& ctx, Ottava* ottava)
e.unknown();
}
}

CompatUtils::resetHookHeightSign(ottava);
}

//---------------------------------------------------------
Expand Down Expand Up @@ -1295,6 +1298,8 @@ static void readTextLine114(XmlReader& e, ReadContext& ctx, TextLine* textLine)
e.unknown();
}
}

CompatUtils::resetHookHeightSign(textLine);
}

//---------------------------------------------------------
Expand Down Expand Up @@ -1388,6 +1393,8 @@ static void readPedal114(XmlReader& e, ReadContext& ctx, Pedal* pedal)
} else if (pedal->endText() == pedal->propertyDefault(Pid::END_TEXT).value<String>()) {
pedal->setPropertyFlags(Pid::END_TEXT, PropertyFlags::STYLED);
}

CompatUtils::resetHookHeightSign(pedal);
}

//---------------------------------------------------------
Expand Down
6 changes: 6 additions & 0 deletions src/engraving/rw/read206/read206.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2193,6 +2193,7 @@ static bool readTextLineProperties(XmlReader& e, ReadContext& ctx, TextLineBase*
} else if (!read400::TRead::readProperties(tl, e, ctx)) {
return false;
}

return true;
}

Expand Down Expand Up @@ -2223,6 +2224,7 @@ static void readVolta206(XmlReader& e, ReadContext& ctx, Volta* volta)
LOGW("Correcting volta anchor type from %d to %d", int(volta->anchor()), int(Volta::VOLTA_ANCHOR));
volta->setAnchor(Volta::VOLTA_ANCHOR);
}
CompatUtils::resetHookHeightSign(volta);
adjustPlacement(volta);
}

Expand Down Expand Up @@ -2268,6 +2270,7 @@ static void readPedal(XmlReader& e, ReadContext& ctx, Pedal* pedal)
pedal->setPropertyFlags(Pid::END_TEXT, PropertyFlags::STYLED);
}

CompatUtils::resetHookHeightSign(pedal);
adjustPlacement(pedal);
}

Expand Down Expand Up @@ -2302,6 +2305,7 @@ static void readOttava(XmlReader& e, ReadContext& ctx, Ottava* ottava)
}
}
ottava->styleChanged();
CompatUtils::resetHookHeightSign(ottava);
adjustPlacement(ottava);
}

Expand Down Expand Up @@ -2344,6 +2348,7 @@ void Read206::readHairpin206(XmlReader& e, ReadContext& ctx, Hairpin* h)
h->setContinueText(u"");
h->setEndText(u"");
}
CompatUtils::resetHookHeightSign(h);
adjustPlacement(h);
}

Expand Down Expand Up @@ -2379,6 +2384,7 @@ void Read206::readTextLine206(XmlReader& e, ReadContext& ctx, TextLineBase* tlb)
e.unknown();
}
}
CompatUtils::resetHookHeightSign(tlb);
adjustPlacement(tlb);
}

Expand Down
8 changes: 8 additions & 0 deletions src/engraving/rw/read400/tread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2652,6 +2652,7 @@ void TRead::read(GradualTempoChange* c, XmlReader& xml, ReadContext& ctx)
xml.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(c);
compat::CompatUtils::setTextLineTextPositionFromAlign(c);
}

Expand Down Expand Up @@ -2701,6 +2702,7 @@ void TRead::read(Hairpin* h, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(h);
compat::CompatUtils::setTextLineTextPositionFromAlign(h);

h->styleChanged();
Expand Down Expand Up @@ -2870,6 +2872,7 @@ void TRead::read(LetRing* r, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(r);
compat::CompatUtils::setTextLineTextPositionFromAlign(r);
}

Expand Down Expand Up @@ -3208,6 +3211,7 @@ void TRead::read(Ottava* o, XmlReader& e, ReadContext& ctx)
while (e.readNextStartElement()) {
readProperties(o, e, ctx);
}
compat::CompatUtils::resetHookHeightSign(o);
compat::CompatUtils::setTextLineTextPositionFromAlign(o);
if (o->ottavaType() != OttavaType::OTTAVA_8VA || o->numbersOnly() != o->propertyDefault(Pid::NUMBERS_ONLY).toBool()) {
o->styleChanged();
Expand Down Expand Up @@ -3274,6 +3278,7 @@ void TRead::read(PalmMute* p, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(p);
compat::CompatUtils::setTextLineTextPositionFromAlign(p);
}

Expand Down Expand Up @@ -3452,6 +3457,7 @@ void TRead::read(Pedal* p, XmlReader& e, ReadContext& ctx)
p->setPropertyFlags(Pid::END_TEXT, PropertyFlags::STYLED);
}

compat::CompatUtils::resetHookHeightSign(p);
compat::CompatUtils::setTextLineTextPositionFromAlign(p);
}

Expand Down Expand Up @@ -3988,6 +3994,7 @@ void TRead::read(TextLineBase* b, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(b);
compat::CompatUtils::setTextLineTextPositionFromAlign(b);
}

Expand Down Expand Up @@ -4343,6 +4350,7 @@ void TRead::read(Volta* v, XmlReader& e, ReadContext& ctx)
}
}

compat::CompatUtils::resetHookHeightSign(v);
compat::CompatUtils::setTextLineTextPositionFromAlign(v);
}

Expand Down
9 changes: 9 additions & 0 deletions src/engraving/rw/read410/tread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2804,6 +2804,7 @@ void TRead::read(GradualTempoChange* c, XmlReader& xml, ReadContext& ctx)
}
}

compat::CompatUtils::resetHookHeightSign(c);
compat::CompatUtils::setTextLineTextPositionFromAlign(c);
}

Expand Down Expand Up @@ -2909,6 +2910,7 @@ void TRead::read(Hairpin* h, XmlReader& e, ReadContext& ctx)
}
}

compat::CompatUtils::resetHookHeightSign(h);
compat::CompatUtils::setTextLineTextPositionFromAlign(h);

h->styleChanged();
Expand Down Expand Up @@ -3077,6 +3079,7 @@ void TRead::read(LetRing* r, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(r);
compat::CompatUtils::setTextLineTextPositionFromAlign(r);
}

Expand Down Expand Up @@ -3406,6 +3409,7 @@ void TRead::read(Ottava* o, XmlReader& e, ReadContext& ctx)
while (e.readNextStartElement()) {
readProperties(o, e, ctx);
}
compat::CompatUtils::resetHookHeightSign(o);
compat::CompatUtils::setTextLineTextPositionFromAlign(o);
if (o->ottavaType() != OttavaType::OTTAVA_8VA || o->numbersOnly() != o->propertyDefault(Pid::NUMBERS_ONLY).toBool()) {
o->styleChanged();
Expand Down Expand Up @@ -3461,6 +3465,7 @@ void TRead::read(PalmMute* p, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(p);
compat::CompatUtils::setTextLineTextPositionFromAlign(p);
}

Expand Down Expand Up @@ -3608,6 +3613,7 @@ void TRead::read(Pedal* p, XmlReader& e, ReadContext& ctx)
p->setPropertyFlags(Pid::END_TEXT, PropertyFlags::STYLED);
}
}
compat::CompatUtils::resetHookHeightSign(p);
compat::CompatUtils::setTextLineTextPositionFromAlign(p);
}

Expand Down Expand Up @@ -4134,6 +4140,8 @@ void TRead::read(TextLineBase* b, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}

compat::CompatUtils::resetHookHeightSign(b);
Copy link
Contributor

@mike-spa mike-spa Mar 5, 2026

Choose a reason for hiding this comment

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

Seems like there's code before the function call that does the same thing as the function?

compat::CompatUtils::setTextLineTextPositionFromAlign(b);
}

Expand Down Expand Up @@ -4470,6 +4478,7 @@ void TRead::read(Volta* v, XmlReader& e, ReadContext& ctx)
e.unknown();
}
}
compat::CompatUtils::resetHookHeightSign(v);
compat::CompatUtils::setTextLineTextPositionFromAlign(v);
}

Expand Down
2 changes: 2 additions & 0 deletions src/engraving/tests/compat114_data/hairpin-ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
Copy link
Contributor

@mike-spa mike-spa Mar 5, 2026

Choose a reason for hiding this comment

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

These tests changes look quite weird: why are we suddenly adding hookHeight tags to hairpins?

Copy link
Contributor Author

@miiizen miiizen Mar 5, 2026

Choose a reason for hiding this comment

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

We've changed the hookHeight value by flipping it so it no longer matches the default. Hairpin's hook heights look like this by default pre 4.6
Screenshot 2026-03-05 at 10 45 40

<eid>O_O</eid>
</HairPin>
<next>
Expand Down
2 changes: 1 addition & 1 deletion src/engraving/tests/compat114_data/pedal-ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
<Spanner type="Pedal">
<Pedal>
<continueText>&lt;font size=&quot;11&quot;/&gt;</continueText>
<endHookHeight>-1.5</endHookHeight>
<endHookHeight>1.5</endHookHeight>
<eid>I_I</eid>
</Pedal>
<next>
Expand Down
4 changes: 4 additions & 0 deletions src/engraving/tests/compat206_data/hairpin-ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>H_H</eid>
</HairPin>
<next>
Expand Down Expand Up @@ -732,6 +734,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>5B_5B</eid>
</HairPin>
<next>
Expand Down
2 changes: 0 additions & 2 deletions src/engraving/tests/compat206_data/textstyles-ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,6 @@
<Spanner type="Pedal">
<Pedal>
<endHookType>1</endHookType>
<beginHookHeight>-1.2</beginHookHeight>
<endHookHeight>-1.2</endHookHeight>
<eid>AB_AB</eid>
</Pedal>
<next>
Expand Down
8 changes: 8 additions & 0 deletions src/engraving/tests/copypaste_data/copypaste_parts-ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>i_i</eid>
</HairPin>
<next>
Expand Down Expand Up @@ -642,6 +644,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>ZB_ZB</eid>
</HairPin>
<next>
Expand Down Expand Up @@ -1110,6 +1114,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>bC_bC</eid>
<linkedTo>i_i</linkedTo>
</HairPin>
Expand Down Expand Up @@ -1516,6 +1522,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>SD_SD</eid>
<linkedTo>ZB_ZB</linkedTo>
</HairPin>
Expand Down
2 changes: 2 additions & 0 deletions src/engraving/tests/implode_explode_data/implodeDynamics01-ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>J_J</eid>
</HairPin>
<next>
Expand Down
2 changes: 2 additions & 0 deletions src/engraving/tests/implode_explode_data/implodeDynamics02-ref.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@
<Spanner type="HairPin">
<HairPin>
<subtype>0</subtype>
<beginHookHeight>-1.9</beginHookHeight>
<endHookHeight>-1.9</endHookHeight>
<eid>J_J</eid>
</HairPin>
<next>
Expand Down
Loading
Loading