From f603cff9bc0f53bd902ce6df5b7f7c3aa85d9eed Mon Sep 17 00:00:00 2001 From: Marc Sabatella Date: Mon, 24 Nov 2014 11:31:36 -0700 Subject: [PATCH] fix #39876: no undo of chord symbol edit --- libmscore/harmony.cpp | 25 ++++++++++++++++++++----- libmscore/harmony.h | 1 + libmscore/text.cpp | 27 ++++++++++++++++++++++----- libmscore/undo.cpp | 2 +- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/libmscore/harmony.cpp b/libmscore/harmony.cpp index 2f6a2f9804e0..c176c3436552 100644 --- a/libmscore/harmony.cpp +++ b/libmscore/harmony.cpp @@ -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(); @@ -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(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(); @@ -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); } //--------------------------------------------------------- @@ -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 //--------------------------------------------------------- diff --git a/libmscore/harmony.h b/libmscore/harmony.h index 534e3d1efb5b..680a4de2620f 100644 --- a/libmscore/harmony.h +++ b/libmscore/harmony.h @@ -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; } diff --git a/libmscore/text.cpp b/libmscore/text.cpp index f86fa02e475c..00694719ef8c 100644 --- a/libmscore/text.cpp +++ b/libmscore/text.cpp @@ -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 @@ -1499,15 +1504,21 @@ 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(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" @@ -1515,7 +1526,13 @@ void Text::endEdit() //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 diff --git a/libmscore/undo.cpp b/libmscore/undo.cpp index 8ec41fa9ada9..222d8041a658 100644 --- a/libmscore/undo.cpp +++ b/libmscore/undo.cpp @@ -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; }