Skip to content

Stave sharing - Part 4 #34036

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
mike-spa wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Stave sharing - Part 4 #34036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e836e50
Format on-staff label the same way as margin labels
mike-spa Jul 2, 2026
d351bfa
Fix shared tuplet error
mike-spa Jul 2, 2026
b001487
Enforce stricter property linking between origin and shared notation
mike-spa Jul 2, 2026
980bcd2
More options to format on-staff labels
mike-spa Jul 3, 2026
288cb40
Add options to restate unison labels after system break
mike-spa Jul 6, 2026
36f8748
codestyle
mike-spa Jul 6, 2026
be91d4e
Fix first-of-next-system bugs
mike-spa Jul 7, 2026
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
14 changes: 14 additions & 0 deletions src/engraving/dom/engravingobject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,20 @@ static void changeProperties(EngravingObject* object, Pid propertyId, const Prop
break;
}
}

if (!object->isEngravingItem() || propertyGroup(propertyId) == PropertyGroup::POSITION) {
return;
}

EngravingItem* item = toEngravingItem(object);
for (EngravingItem* originItem : item->originItems()) {
// This is a shared item: propagate to all origin items
changeProperty(originItem, propertyId, propertyValue, propertyFlag);
}
if (EngravingItem* sharedItem = item->sharedItem(); sharedItem && sharedItem->originItems().front() == item) {
// This is the first origin item of the shared item: propagate to shared item
changeProperty(sharedItem, propertyId, propertyValue, propertyFlag);
}
}

//---------------------------------------------------------
Expand Down
9 changes: 9 additions & 0 deletions src/engraving/dom/textbase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3387,6 +3387,15 @@ void TextBase::undoChangeProperty(Pid id, const PropertyValue& v, PropertyFlags
break;
}
}

for (EngravingItem* originItem : originItems()) {
// This is a shared item: propagate to all origin items
score()->undo(new ChangeTextProperties(toTextBase(originItem)->cursor(), id, v, ps));
}
if (EngravingItem* sharedEl = sharedItem(); sharedEl && sharedEl->originItems().front() == this) {
// This is the first origin item of the shared item: propagate to shared item
score()->undo(new ChangeTextProperties(toTextBase(sharedEl)->cursor(), id, v, ps));
}
Comment on lines +3391 to +3398

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use the target item's score for shared text undo commands.

The linked-object path above uses linkedText->score(), but this new shared-item path records undo commands on this->score() while targeting another item's cursor. Route each command through the target item’s score to keep the undo transaction attached to the edited object’s owning score.

Proposed fix 
     for (EngravingItem* originItem : originItems()) {
         // This is a shared item: propagate to all origin items
-        score()->undo(new ChangeTextProperties(toTextBase(originItem)->cursor(), id, v, ps));
+        originItem->score()->undo(new ChangeTextProperties(toTextBase(originItem)->cursor(), id, v, ps));
     }
     if (EngravingItem* sharedEl = sharedItem(); sharedEl && sharedEl->originItems().front() == this) {
         // This is the first origin item of the shared item: propagate to shared item
-        score()->undo(new ChangeTextProperties(toTextBase(sharedEl)->cursor(), id, v, ps));
+        sharedEl->score()->undo(new ChangeTextProperties(toTextBase(sharedEl)->cursor(), id, v, ps));
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (EngravingItem* originItem : originItems()) {
// This is a shared item: propagate to all origin items
score()->undo(new ChangeTextProperties(toTextBase(originItem)->cursor(), id, v, ps));
}
if (EngravingItem* sharedEl = sharedItem(); sharedEl && sharedEl->originItems().front() == this) {
// This is the first origin item of the shared item: propagate to shared item
score()->undo(new ChangeTextProperties(toTextBase(sharedEl)->cursor(), id, v, ps));
}
for (EngravingItem* originItem : originItems()) {
// This is a shared item: propagate to all origin items
originItem->score()->undo(new ChangeTextProperties(toTextBase(originItem)->cursor(), id, v, ps));
}
if (EngravingItem* sharedEl = sharedItem(); sharedEl && sharedEl->originItems().front() == this) {
// This is the first origin item of the shared item: propagate to shared item
sharedEl->score()->undo(new ChangeTextProperties(toTextBase(sharedEl)->cursor(), id, v, ps));
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/engraving/dom/textbase.cpp` around lines 3390 - 3397, The shared-text
undo path in TextBase::setProperty is recording ChangeTextProperties on
this->score() even when the cursor belongs to a different origin/shared item.
Update the loop over originItems() and the sharedItem() propagation so each undo
command is submitted through the target item’s owning score, matching the
linkedText->score() behavior and keeping the undo transaction attached to the
edited object.

}

bool mu::engraving::TextBase::hasSymbolScale() const
Expand Down
109 changes: 80 additions & 29 deletions src/engraving/rendering/score/stavesharinglayout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ void StaveSharingLayout::updateStaveSharingForFullSystem(MeasureBase* firstMB, M
updateStaveSharing(ssctx);
}

void StaveSharingLayout::updateStaveSharingForLastAddedMeasure(System* system, LayoutContext& ctx)
bool StaveSharingLayout::updateStaveSharingForLastAddedMeasure(System* system, LayoutContext& ctx)
{
if (!system->last()->isMeasure()) {
return;
return false;
}

StaveSharingContext ssctx(system->first(), system->last(), ctx);
if (ssctx.crSegments.empty()) {
return;
return false;
}

ssctx.updateForLastAdded = true;
Expand All @@ -68,26 +68,48 @@ void StaveSharingLayout::updateStaveSharingForLastAddedMeasure(System* system, L
if (ssctx.trackMapChanged) {
SystemHeaderLayout::updateSystemHeaderWidth(system, ctx);

for (MeasureBase* mb = system->first(); mb && mb != system->last(); mb = mb->next()) {
for (MeasureBase* mb = system->first(); mb; mb = mb->next()) {
if (mb->isMeasure()) {
// TODO: only relayout shared staves
toMeasure(mb)->mutldata()->setNeedLayout(true);
// TODO: only relayout shared staves
MeasureLayout::layoutMeasure(mb, ctx);
}
if (mb == system->last()) {
break;
}
}
}

return ssctx.trackMapChanged;
}

void StaveSharingLayout::updateNotationWithoutRecomputingTrackMap(Measure* measure, LayoutContext& ctx)
{
StaveSharingContext ssctx(measure, measure, ctx);
if (ssctx.crSegments.empty()) {
return;
}

for (Part* p : ssctx.layoutCtx.dom().parts()) {
if (p->isSharedPart() && toSharedPart(p)->show()) {
SharedPart* sharedPart = toSharedPart(p);
ssctx.curSharedPart = sharedPart;
updateNotation(ssctx);
}
}

measure->mutldata()->setNeedLayout(true);
MeasureLayout::layoutMeasure(measure, ctx);
}

void StaveSharingLayout::updateStaveSharing(StaveSharingContext& ctx)
{
for (Part* p : ctx.layoutCtx.dom().parts()) {
if (p->isSharedPart() && p->show() && toSharedPart(p)->enabled()) {
if (p->isSharedPart() && toSharedPart(p)->show()) {
SharedPart* sharedPart = toSharedPart(p);
if (sharedPart->show() && sharedPart->enabled()) {
ctx.curSharedPart = sharedPart;
updateTrackMaps(ctx);
updateNotation(ctx);
}
ctx.curSharedPart = sharedPart;
updateTrackMaps(ctx);
updateNotation(ctx);
}
}
}
Expand Down Expand Up @@ -548,7 +570,7 @@ bool StaveSharingLayout::checkArticulationsForSameVoice(Chord* chord1, Chord* ch
bool StaveSharingLayout::canGoToSameStave(track_idx_t prevTrack, track_idx_t nextTrack,
StaveSharingContext& ctx)
{
if (ctx.layoutCtx.conf().styleB(Sid::allowVoiceCrossing)) {
if (ctx.style.styleB(Sid::allowVoiceCrossing)) {
return true;
}

Expand Down Expand Up @@ -744,8 +766,6 @@ void StaveSharingLayout::makeSharedChordRests(StaveSharingContext& ctx)
sharedTuplet->setTrack(sharedTrack);
sharedTuplet->setParent(originTuplet->measure());
score->undoAddElement(sharedTuplet);

EngravingItem::connectSharedItem(sharedTuplet, originTuplet);
}
} else {
sharedTuplet = toTuplet(originTuplet->sharedItem());
Expand All @@ -755,6 +775,8 @@ void StaveSharingLayout::makeSharedChordRests(StaveSharingContext& ctx)
break;
}

EngravingItem::connectSharedItem(sharedTuplet, originTuplet);

sharedTuplet->add(sharedDE);

originDE = originTuplet;
Expand Down Expand Up @@ -1023,11 +1045,12 @@ void StaveSharingLayout::makeStaveSharingLabels(StaveSharingContext& ctx)
std::vector<EngravingItem*> updatedStaveSharingLabels;

for (Note* unisonNote : ctx.sharedUnisonNotes) {
if (!unisonNoteNeedsLabel(unisonNote)) {
bool isForNewSystem = false;
if (!unisonNoteNeedsLabel(unisonNote, isForNewSystem, ctx)) {
continue;
}

String text = formatUnisonLabel(unisonNote, trackMap, ctx);
String text = formatUnisonLabel(unisonNote, trackMap, isForNewSystem, ctx);
Segment* segment = unisonNote->chord()->segment();

StaveSharingLabel* label = nullptr;
Expand Down Expand Up @@ -1059,8 +1082,9 @@ void StaveSharingLayout::makeStaveSharingLabels(StaveSharingContext& ctx)
}
}

bool StaveSharingLayout::unisonNoteNeedsLabel(Note* unisonNote)
bool StaveSharingLayout::unisonNoteNeedsLabel(Note* unisonNote, bool& isForNewSystem, StaveSharingContext& ctx)
{
bool restateOnNewSystem = ctx.style.styleV(Sid::unisonLabelRestateOnNewSystem).value<AutoOnOff>() != AutoOnOff::OFF;
std::vector<track_idx_t> originTracksOfThisNote;
for (EngravingItem* originNote : unisonNote->originItems()) {
originTracksOfThisNote.push_back(originNote->track());
Expand All @@ -1070,6 +1094,10 @@ bool StaveSharingLayout::unisonNoteNeedsLabel(Note* unisonNote)
track_idx_t curTrack = unisonNote->track();
Segment* curSegment = unisonNote->chord()->segment();
for (Segment* seg = curSegment->prev1(SegmentType::ChordRest); seg; seg = seg->prev1(SegmentType::ChordRest)) {
if (seg->system() != curSegment->system() && restateOnNewSystem) {
isForNewSystem = true;
return true;
}
if (EngravingItem* el = seg->element(curTrack); el && el->isChord()) {
for (Note* note : toChord(el)->notes()) {
if (!note->originItems().empty()) {
Expand All @@ -1095,12 +1123,15 @@ bool StaveSharingLayout::unisonNoteNeedsLabel(Note* unisonNote)
return originTracksOfPrevNote != originTracksOfThisNote;
}

String StaveSharingLayout::formatUnisonLabel(Note* unisonNote, const SharedTrackMap& trackMap, const StaveSharingContext& ctx)
String StaveSharingLayout::formatUnisonLabel(Note* unisonNote, const SharedTrackMap& trackMap, bool isForNewSystem,
const StaveSharingContext& ctx)
{
const MStyle& style = ctx.style;

String result;

size_t originUnisonsCount = unisonNote->originItems().size();
result += u"a " + String::number(originUnisonsCount);
result += style.styleSt(Sid::textForUnisonLabel) + String::number(originUnisonsCount);
bool trailingDot = false; // TODO: style
if (trailingDot) {
result += '.';
Expand All @@ -1111,6 +1142,8 @@ String StaveSharingLayout::formatUnisonLabel(Note* unisonNote, const SharedTrack

std::vector<track_idx_t> originTracks;
originTracks.reserve(originUnisonsCount);
std::vector<Instrument*> originInstruments;
originInstruments.reserve(originUnisonsCount);
std::vector<track_idx_t> tracksMappedToThisStave;
tracksMappedToThisStave.reserve(originUnisonsCount);

Expand All @@ -1120,24 +1153,42 @@ String StaveSharingLayout::formatUnisonLabel(Note* unisonNote, const SharedTrack
}
if (sharedTrack == curTrack) {
originTracks.push_back(originTrack);
originInstruments.push_back(ctx.score->staff(track2staff(originTrack))->part()->instrument());
}
}

bool addParenthesis = isForNewSystem && ctx.style.styleV(Sid::unisonLabelRestateOnNewSystem).value<AutoOnOff>() == AutoOnOff::AUTO;

if (tracksMappedToThisStave.size() <= originUnisonsCount) {
if (addParenthesis) {
result.prepend('(');
result.append(')');
}
return result;
}

String prefix;
for (track_idx_t originTrack : originTracks) {
if (!prefix.empty()) {
prefix += '.';
}
Instrument* originInstrument = ctx.score->staff(track2staff(originTrack))->part()->instrument();
prefix += String::number(originInstrument->number());
bool trailingDotSingle;
bool trailingDotMultiple;
int hyphenLimit;
if (style.styleB(Sid::sharedOnStaffNumeralsFollowInstrumentNumerals)) {
trailingDotSingle = style.styleB(Sid::instrumentNumeralsTrailingDotSingle);
trailingDotMultiple = style.styleB(Sid::instrumentNumeralsTrailingDotMultiple);
hyphenLimit = style.styleB(Sid::instrumentNumeralsHyphenEnable) ? style.styleI(Sid::instrumentNumeralsHyphenThreshold) : INT_MAX;
} else {
trailingDotSingle = style.styleB(Sid::sharedOnStaffNumeralsTrailingDotSingle);
trailingDotMultiple = style.styleB(Sid::sharedOnStaffNumeralsTrailingDotMultiple);
hyphenLimit = style.styleB(Sid::sharedOnStaffNumeralsHyphenEnable)
? style.styleI(Sid::sharedOnStaffNumeralsHyphenThreshold) : INT_MAX;
}
prefix += ' ';

result.prepend(prefix);
String prefix = SystemHeaderLayout::formatSharedVoiceLabel(originInstruments, trailingDotSingle, trailingDotMultiple, hyphenLimit);

result.prepend(prefix + ' ');

if (addParenthesis) {
result.prepend('(');
result.append(')');
}

return result;
}
Expand Down Expand Up @@ -1282,7 +1333,7 @@ void StaveSharingLayout::cleanup(StaveSharingContext& ctx)
}

StaveSharingLayout::StaveSharingContext::StaveSharingContext(MeasureBase* first, MeasureBase* last, LayoutContext& ctx)
: layoutCtx(ctx)
: layoutCtx(ctx), style(ctx.conf().style())
{
for (MeasureBase* mb = first; mb; mb = mb->next()) {
if (!mb->isMeasure()) {
Expand Down
8 changes: 5 additions & 3 deletions src/engraving/rendering/score/stavesharinglayout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class StaveSharingLayout
{
public:
static void updateStaveSharingForFullSystem(MeasureBase* firstMB, MeasureBase* lastMB, LayoutContext& ctx);
static void updateStaveSharingForLastAddedMeasure(System* system, LayoutContext& ctx);
static bool updateStaveSharingForLastAddedMeasure(System* system, LayoutContext& ctx);
static void updateNotationWithoutRecomputingTrackMap(Measure* measure, LayoutContext& ctx);

private:
using TrackGroup = std::vector<track_idx_t>;
Expand All @@ -54,6 +55,7 @@ class StaveSharingLayout

Score* score = nullptr;
LayoutContext& layoutCtx;
const MStyle& style;

StaveSharingContext(MeasureBase* first, MeasureBase* last, LayoutContext& ctx);
};
Expand Down Expand Up @@ -88,8 +90,8 @@ class StaveSharingLayout
static void makeSharedSpanners(StaveSharingContext& ctx);

static void makeStaveSharingLabels(StaveSharingContext& ctx);
static bool unisonNoteNeedsLabel(Note* unisonNote);
static String formatUnisonLabel(Note* unisonNote, const SharedTrackMap& trackMap, const StaveSharingContext& ctx);
static bool unisonNoteNeedsLabel(Note* unisonNote, bool& isForNewSystem, StaveSharingContext& ctx);
static String formatUnisonLabel(Note* unisonNote, const SharedTrackMap& trackMap, bool isForNewSystem, const StaveSharingContext& ctx);

static void manageVoicePropertyAndTrackForSharedItems(const std::vector<EngravingItem*>& sharedItems, track_idx_t startOriginTrack,
track_idx_t endOriginTrack, const SharedTrackMap& trackMap);
Expand Down
18 changes: 6 additions & 12 deletions src/engraving/rendering/score/systemheaderlayout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,21 +1189,21 @@ String SystemHeaderLayout::formattedSharedStaffLabel(staff_idx_t staffIdx, const
String result;

if (actualOrientation == SharedLabelOrientation::HORIZONTAL) {
result = formatVoice(instrumentsMappedToFirstVoice, /*isFirstVoice*/ true, trailingDotSingle, trailingDotMultiple, hyphenLimit);
result = formatSharedVoiceLabel(instrumentsMappedToFirstVoice, trailingDotSingle, trailingDotMultiple, hyphenLimit);
} else {
result = formatVerticalSharedLabel(instrumentsMappedToFirstVoice, trailingDotSingle);
}

return result;
}

String result = formatVoice(instrumentsMappedToFirstVoice, /*isFirstVoice*/ true, trailingDotSingle, trailingDotMultiple, hyphenLimit);
String result = formatSharedVoiceLabel(instrumentsMappedToFirstVoice, trailingDotSingle, trailingDotMultiple, hyphenLimit);

if (instrumentsMappedToSecondVoice.empty()) {
return result;
}

result += formatVoice(instrumentsMappedToSecondVoice, /*isFirstVoice*/ false, trailingDotSingle, trailingDotMultiple, hyphenLimit);
result += '\n' + formatSharedVoiceLabel(instrumentsMappedToSecondVoice, trailingDotSingle, trailingDotMultiple, hyphenLimit);

return result;
}
Expand All @@ -1227,22 +1227,16 @@ String SystemHeaderLayout::formatVerticalSharedLabel(const std::vector<Instrumen
return result;
}

String SystemHeaderLayout::formatVoice(const std::vector<Instrument*>& instruments, bool isFirstVoice, bool trailingDotSingle,
bool trailingDotMultiple, int hyphenLimit)
String SystemHeaderLayout::formatSharedVoiceLabel(const std::vector<Instrument*>& instruments, bool trailingDotSingle,
bool trailingDotMultiple, int hyphenLimit)
{
String result;

if (!isFirstVoice) {
result += '\n';
}

size_t voiceCount = instruments.size();
bool putTrailingDot = (voiceCount <= 1 && trailingDotSingle) || (voiceCount > 1 && trailingDotMultiple);

for (size_t i = 0; i < instruments.size(); ++i) {
if (isFirstVoice && !result.empty()) {
result += '.';
} else if (!isFirstVoice && result.back() != '\n') {
if (!result.empty() && result.back() != '\n') {
result += '.';
}

Expand Down
5 changes: 3 additions & 2 deletions src/engraving/rendering/score/systemheaderlayout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ class SystemHeaderLayout
static void setGroupBracketsHorizontalPos(System* system);
static void setInstrumentNames(System* system, LayoutContext& ctx);

static String formatSharedVoiceLabel(const std::vector<Instrument*>& instruments, bool trailingDotSingle, bool trailingDotMultiple,
int hyphenLimit);

private:
static Bracket* createBracket(System* system, LayoutContext& ctx, BracketItem* bi, size_t column, staff_idx_t staffIdx,
std::vector<Bracket*>& bl, Measure* measure);
Expand All @@ -72,8 +75,6 @@ class SystemHeaderLayout

static String formattedSharedStaffLabel(staff_idx_t staffIdx, const SharedTrackMap& trackMap, const std::vector<Part*>& originParts);
static String formatVerticalSharedLabel(const std::vector<Instrument*>& instruments, bool trailingDotSingle);
static String formatVoice(const std::vector<Instrument*>& instruments, bool isFirstVoice, bool trailingDotSingle,
bool trailingDotMultiple, int hyphenLimit);

static String& resolveTokens(String& str, const String& name, const String& transposition, const String& number);
static bool showNames(LayoutContext& ctx);
Expand Down
Loading
Loading