Skip to content

Commit

Permalink
fix #39876: no undo of chord symbol edit
Browse files Browse the repository at this point in the history
  • Loading branch information
MarcSabatella committed Nov 25, 2014
1 parent 0a59df0 commit f603cff
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 11 deletions.
25 changes: 20 additions & 5 deletions libmscore/harmony.cpp
Expand Up @@ -597,7 +597,6 @@ void Harmony::startEdit(MuseScoreView* view, const QPointF& p)
if (!textList.isEmpty()) {
QString s(harmonyName());
setText(s);
Text::createLayout(); // create TextBlocks from text
}
Text::startEdit(view, p);
layout();
Expand Down Expand Up @@ -626,15 +625,16 @@ bool Harmony::edit(MuseScoreView* view, int grip, int key, Qt::KeyboardModifiers
void Harmony::endEdit()
{
Text::endEdit();
setHarmony(plainText(true));
layout();
if (links()) {
foreach(Element* e, *links()) {
if (e == this)
continue;
Harmony* h = static_cast<Harmony*>(e);
h->setHarmony(text());
// transpose if necessary
// at this point chord will already have been rendered in same key as original
// (as a result of Text::endEdit() calling setText() for linked elements)
// we may now need to change the TPC's and the text, and re-render
if (score()->styleB(StyleIdx::concertPitch) != h->score()->styleB(StyleIdx::concertPitch)) {
Part* partDest = h->staff()->part();
Interval interval = partDest->instr()->transpose();
Expand All @@ -643,12 +643,16 @@ void Harmony::endEdit()
interval.flip();
int rootTpc = transposeTpc(h->rootTpc(), interval, false);
int baseTpc = transposeTpc(h->baseTpc(), interval, false);
h->score()->undoTransposeHarmony(h, rootTpc, baseTpc);
//score()->undoTransposeHarmony(h, rootTpc, baseTpc);
h->setRootTpc(rootTpc);
h->setBaseTpc(baseTpc);
// this invokes textChanged(), which handles the rendering
h->setText(h->harmonyName());
}
}
}
}
score()->setLayoutAll(true); // done in Text::endEdit() too, but no harm being sure
score()->setLayoutAll(true);
}

//---------------------------------------------------------
Expand Down Expand Up @@ -889,6 +893,17 @@ bool Harmony::isEmpty() const
return textList.isEmpty() && Text::isEmpty();
}

//---------------------------------------------------------
// textChanged
// text may have changed
//---------------------------------------------------------

void Harmony::textChanged()
{
Text::createLayout();
setHarmony(plainText(true));
}

//---------------------------------------------------------
// layout
//---------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions libmscore/harmony.h
Expand Up @@ -122,6 +122,7 @@ class Harmony : public Text {

void determineRootBaseSpelling(NoteSpellingType& rootSpelling, bool& rootLowerCase, NoteSpellingType& baseSpelling, bool& baseLowerCase);

virtual void textChanged() override;
virtual void layout();

const QRectF& bboxtight() const { return _tbbox; }
Expand Down
27 changes: 22 additions & 5 deletions libmscore/text.cpp
Expand Up @@ -1488,7 +1488,12 @@ void Text::endEdit()

genText();

if (_text != oldText) {
if (_text != oldText || type() == Element::Type::HARMONY) {
// avoid creating unnecessary state on undo stack if edit did not change anything
// but go ahead and do this anyhow for chord symbols no matter what
// the code to special case transposition relies on the fact
// that we are setting all linked elements to same text here

for (Element* e : linkList()) {
// this line was added in https://github.com/musescore/MuseScore/commit/dcf963b3d6140fa550c08af18d9fb6f6e59733a3
// it replaced the commented-out call to undoPushProperty in startEdit() above
Expand All @@ -1499,23 +1504,35 @@ void Text::endEdit()
// by also checking for empty old text, we avoid creating an unnecessary element on undo stack
// that returns us to the initial empty text created upon startEdit()

if (!oldText.isEmpty())
score()->undo()->push1(new ChangeProperty(e, P_ID::TEXT, oldText));
if (!oldText.isEmpty()) {
// oldText is good for original element
// but use original text for each linked element
// these can differ (eg, for chord symbols in transposing parts)

QString undoText = (e == this) ? oldText : static_cast<Text*>(e)->_text;
score()->undo()->push1(new ChangeProperty(e, P_ID::TEXT, undoText));
}

// because we are pushing each individual linked element's old text to the undo stack,
// we don't actually need to call the undo version of change property here

e->setProperty(P_ID::TEXT, _text);

// the change mentioned above eliminated the following line, which is where the linked elements actually got their text set
// the change mentioned previously eliminated the following line, which is where the linked elements actually got their text set
// one would think this line alone would be enough to make undo work
// but it is not, because by the time we get here, we've already overwritten _text for the current item
// that is why formerly we skipped this call for "this"
// and this was safe because we formerly pushed the old text for "this" back in startEdit()
//if (e != this) e->undoChangeProperty(P_ID::TEXT, _text);
}
}
textChanged();
else {
// only necessary in the case of _text == oldtext
// because otherwise, setProperty() call above calls setText(), which calls textChanged()
// yet we still need to consider this a change, since newly added palette texts end up here
textChanged();
}

// formerly we needed to setLayoutAll here to force the text to be laid out after editing
// but now that we are calling setProperty for all elements - including "this"
// it is no longer necessary
Expand Down
2 changes: 1 addition & 1 deletion libmscore/undo.cpp
Expand Up @@ -2247,7 +2247,7 @@ void TransposeHarmony::flip()
int rootTpc1 = harmony->rootTpc();
harmony->setBaseTpc(baseTpc);
harmony->setRootTpc(rootTpc);
harmony->render();
harmony->setText(harmony->harmonyName());
rootTpc = rootTpc1;
baseTpc = baseTpc1;
}
Expand Down

0 comments on commit f603cff

Please sign in to comment.