From 8d25cea5171e901131a5b34b1cd0278ca26dee79 Mon Sep 17 00:00:00 2001 From: David Stephen Grant Date: Tue, 27 Oct 2026 23:08:39 +0100 Subject: [PATCH] Move adding tuplets to the selection into EditTuplet NotationInteraction::addTupletToSelectedChordRests() gathered the selected chords/rests, divided them into tuplets and set the selection afterwards, out of reach of the engraving tests. Move this unchanged into the new EditTuplet::addTuplets(), which returns the tuplets in score order, and test it. Assisted-by: Claude Code 2.1.310 (Claude Opus 5.5) --- src/engraving/CMakeLists.txt | 2 + src/engraving/editing/edittuplet.cpp | 128 ++++++++++++++++ src/engraving/editing/edittuplet.h | 42 ++++++ src/engraving/tests/tuplet_tests.cpp | 140 ++++++++++++++++++ src/notation/internal/notationinteraction.cpp | 67 +-------- 5 files changed, 316 insertions(+), 63 deletions(-) create mode 100644 src/engraving/editing/edittuplet.cpp create mode 100644 src/engraving/editing/edittuplet.h diff --git a/src/engraving/CMakeLists.txt b/src/engraving/CMakeLists.txt index 59406f29c153..262d662b647d 100644 --- a/src/engraving/CMakeLists.txt +++ b/src/engraving/CMakeLists.txt @@ -186,6 +186,8 @@ target_sources(engraving PRIVATE editing/edittimesig.h editing/edittremolo.cpp editing/edittremolo.h + editing/edittuplet.cpp + editing/edittuplet.h editing/editvisibility.cpp editing/editvisibility.h editing/editvoice.cpp diff --git a/src/engraving/editing/edittuplet.cpp b/src/engraving/editing/edittuplet.cpp new file mode 100644 index 000000000000..196273da7e7d --- /dev/null +++ b/src/engraving/editing/edittuplet.cpp @@ -0,0 +1,128 @@ +/* + * SPDX-License-Identifier: GPL-3.0-only + * MuseScore-Studio-CLA-applies + * + * MuseScore Studio + * Music Composition & Notation + * + * Copyright (C) 2026 MuseScore Limited and others + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "edittuplet.h" + +#include +#include + +#include "../dom/chord.h" +#include "../dom/chordrest.h" +#include "../dom/input.h" +#include "../dom/note.h" +#include "../dom/score.h" +#include "../dom/select.h" + +#include "log.h" + +using namespace mu::engraving; + +static Fraction dividingRatio(const ChordRest* chordRest, const Fraction& ratio, bool autoBaseLen) +{ + Fraction result = ratio; + if (autoBaseLen) { + result.setDenominator(Tuplet::computeTupletDenominator(ratio.numerator(), chordRest->ticks())); + } + return result; +} + +// Selected chords/rests a tuplet can start at, in score order +static std::vector selectedTupletChordRests(const Score* score) +{ + // Score order, not pointer order + const std::set selectedChordRests = score->getSelectedChordRests(); + std::vector sortedChordRests(selectedChordRests.begin(), selectedChordRests.end()); + std::sort(sortedChordRests.begin(), sortedChordRests.end(), [](const ChordRest* a, const ChordRest* b) { + return a->tick() != b->tick() ? a->tick() < b->tick() : a->track() < b->track(); + }); + + std::vector chordRests; + + // Make sure each linked group is handled only once + std::unordered_set handledLinks; + + for (ChordRest* chordRest : sortedChordRests) { + if (chordRest->links() && !handledLinks.insert(chordRest->links()).second) { + continue; + } + + if (!chordRest->isGrace() && !(chordRest->isChord() && toChord(chordRest)->isTrillCueNote())) { + chordRests.push_back(chordRest); + } + } + + return chordRests; +} + +static void selectTupletStart(Score* score, const Tuplet* tuplet) +{ + if (tuplet->elements().empty()) { + return; + } + + DurationElement* firstElement = tuplet->elements().front(); + // SINGLE also moves the input position + if (firstElement->isChord()) { + const std::vector& notes = toChord(firstElement)->notes(); + IF_ASSERT_FAILED(!notes.empty()) { + score->select(firstElement, SelectType::SINGLE); + } else { + score->select(notes.front(), SelectType::SINGLE); + score->select({ notes.begin() + 1, notes.end() }, SelectType::ADD); + } + } else { + score->select(firstElement, SelectType::SINGLE); + } +} + +std::vector EditTuplet::addTuplets(Transaction&, Score* score, const Fraction& ratio, TupletNumberType numberType, + TupletBracketType bracketType, bool autoBaseLen) +{ + Selection& selection = score->selection(); + const bool wasRangeSelection = selection.isRange(); + const Fraction startTick = selection.tickStart(); + const Fraction endTick = selection.tickEnd(); + const staff_idx_t startStaff = selection.staffStart(); + const staff_idx_t endStaff = selection.staffEnd(); + Box* startBox = selection.startBox(); + Box* endBox = selection.endBox(); + + std::vector tuplets; + for (ChordRest* chordRest : selectedTupletChordRests(score)) { + if (Tuplet* tuplet = score->addTuplet(chordRest, dividingRatio(chordRest, ratio, autoBaseLen), numberType, bracketType)) { + tuplets.push_back(tuplet); + } + } + + if (wasRangeSelection) { + selection.setRangeTicks(startTick, endTick, startStaff, endStaff, startBox, endBox); + selection.updateSelectedElements(); + } else if (!tuplets.empty()) { + selectTupletStart(score, tuplets.front()); + } + + if (!tuplets.empty()) { + score->inputState().setDuration(tuplets.front()->baseLen()); + } + + return tuplets; +} diff --git a/src/engraving/editing/edittuplet.h b/src/engraving/editing/edittuplet.h new file mode 100644 index 000000000000..91554150a13f --- /dev/null +++ b/src/engraving/editing/edittuplet.h @@ -0,0 +1,42 @@ +/* + * SPDX-License-Identifier: GPL-3.0-only + * MuseScore-Studio-CLA-applies + * + * MuseScore Studio + * Music Composition & Notation + * + * Copyright (C) 2026 MuseScore Limited and others + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#pragma once + +#include + +#include "../dom/tuplet.h" + +namespace mu::engraving { +class Fraction; +class Score; +class Transaction; + +class EditTuplet +{ +public: + // Divides each selected chord/rest into a tuplet (3:2 on a quarter: 3 eighths in the time of a quarter). Keeps a range + // selection, otherwise selects the start of the first tuplet. Returns the tuplets in score order + static std::vector addTuplets(Transaction& tx, Score* score, const Fraction& ratio, TupletNumberType numberType, + TupletBracketType bracketType, bool autoBaseLen); +}; +} diff --git a/src/engraving/tests/tuplet_tests.cpp b/src/engraving/tests/tuplet_tests.cpp index a2c2c530ce17..e2aa87c3d5c6 100644 --- a/src/engraving/tests/tuplet_tests.cpp +++ b/src/engraving/tests/tuplet_tests.cpp @@ -23,6 +23,7 @@ #include #include "engraving/dom/chord.h" +#include "engraving/dom/excerpt.h" #include "engraving/dom/factory.h" #include "engraving/dom/masterscore.h" #include "engraving/dom/measure.h" @@ -31,6 +32,7 @@ #include "engraving/dom/staff.h" #include "engraving/dom/timesig.h" #include "engraving/dom/tuplet.h" +#include "engraving/editing/edittuplet.h" #include "engraving/editing/noteinput.h" #include "engraving/editing/splitjoinmeasure.h" #include "engraving/editing/transaction/transaction.h" @@ -723,6 +725,144 @@ TEST_F(Engraving_TupletTests, addTupletKeepsSelection) delete score; } +// Divides the selected chords/rests, as the tuplet commands do +static std::vector addTuplets(Score* score, const Fraction& ratio) +{ + std::vector tuplets; + score->transactionManager()->transaction(TranslatableString::untranslatable("Engraving tuplet tests"), [&](Transaction& tx) { + tuplets = EditTuplet::addTuplets(tx, score, ratio, TupletNumberType::SHOW_NUMBER, TupletBracketType::AUTO_BRACKET, + /*autoBaseLen*/ true); + }); + return tuplets; +} + +// Adds a second staff to the first staff's part, linked to it or with measure rests +static void addSecondStaff(Score* score, bool linked) +{ + score->startCmd(TranslatableString::untranslatable("Engraving tuplet tests")); + Staff* staff = score->staff(0); + Staff* newStaff = Factory::createStaff(staff->part()); + newStaff->setPart(staff->part()); + score->undoInsertStaff(newStaff, 1, /*createRests*/ !linked); + if (linked) { + Excerpt::cloneStaff(staff, newStaff); + } + score->endCmd(); +} + +TEST_F(Engraving_TupletTests, addTuplets) +{ + MasterScore* score = ScoreRW::readScore(TUPLET_DATA_DIR + u"addTuplet.mscx"); + ASSERT_TRUE(score); + Measure* m1 = measureAt(score, 0); + + // 3 eighths, selected from last to first: the tuplets come back in score order + score->select(toChord(chordRestAt(m1, 5))->upNote(), SelectType::SINGLE); + score->select(toChord(chordRestAt(m1, 2))->upNote(), SelectType::ADD); + score->select(toChord(chordRestAt(m1, 0))->upNote(), SelectType::ADD); + const std::vector tuplets = addTuplets(score, Fraction(3, 2)); + ASSERT_EQ(tuplets.size(), 3); + EXPECT_EQ(tuplets[0]->tick(), Fraction(0, 1)); + EXPECT_EQ(tuplets[1]->tick(), Fraction(1, 4)); + EXPECT_EQ(tuplets[2]->tick(), Fraction(5, 8)); + EXPECT_TRUE(score->sanityCheck()); + EXPECT_EQ(rhythm(m1), "3:2[16 r16 r16] 8 3:2[16 r16 r16] 8 8 3:2[16 r16 r16] 8 8"); + + // the start of the first tuplet is selected, and its note value becomes the input duration + EXPECT_TRUE(score->selection().isSingle()); + EXPECT_EQ(score->selection().element(), toChord(tuplets[0]->elements().front())->upNote()); + EXPECT_EQ(score->inputState().duration(), TDuration(DurationType::V_16TH)); + + score->undoRedo(true, nullptr); + EXPECT_EQ(rhythm(m1), "8 8 8 8 8 8 8 8"); + + delete score; +} + +TEST_F(Engraving_TupletTests, addTupletsKeepsRange) +{ + MasterScore* score = ScoreRW::readScore(TUPLET_DATA_DIR + u"addTuplet.mscx"); + ASSERT_TRUE(score); + Measure* m1 = measureAt(score, 0); + + score->select(chordRestAt(m1, 0), SelectType::RANGE); + score->select(chordRestAt(m1, 1), SelectType::RANGE); + ASSERT_EQ(score->getSelectedChordRests().size(), 2); + EXPECT_EQ(addTuplets(score, Fraction(3, 2)).size(), 2); + EXPECT_TRUE(score->sanityCheck()); + EXPECT_EQ(rhythm(m1), "3:2[16 r16 r16] 3:2[16 r16 r16] 8 8 8 8 8 8"); + + const Selection& selection = score->selection(); + EXPECT_TRUE(selection.isRange()); + EXPECT_EQ(selection.tickStart(), Fraction(0, 1)); + EXPECT_EQ(selection.tickEnd(), Fraction(1, 4)); + EXPECT_EQ(selection.staffStart(), 0); + EXPECT_EQ(selection.staffEnd(), 1); + EXPECT_EQ(score->getSelectedChordRests().size(), 6); + + delete score; +} + +TEST_F(Engraving_TupletTests, addTupletsRangeAcrossStaves) +{ + MasterScore* score = ScoreRW::readScore(TUPLET_DATA_DIR + u"addTuplet.mscx"); + ASSERT_TRUE(score); + addSecondStaff(score, /*linked*/ false); + Measure* m1 = measureAt(score, 0); + + // the first 2 eighths of m1 and the measure rest below them + score->selection().setRange(chordRestAt(m1, 0)->segment(), chordRestAt(m1, 2)->segment(), 0, 2); + score->selection().updateSelectedElements(); + ASSERT_EQ(score->getSelectedChordRests().size(), 3); + const std::vector tuplets = addTuplets(score, Fraction(3, 2)); + EXPECT_TRUE(score->sanityCheck()); + + // score order: by tick, then by track + ASSERT_EQ(tuplets.size(), 3); + EXPECT_EQ(tuplets[0]->tick(), Fraction(0, 1)); + EXPECT_EQ(tuplets[0]->staffIdx(), 0); + EXPECT_EQ(tuplets[1]->tick(), Fraction(0, 1)); + EXPECT_EQ(tuplets[1]->staffIdx(), 1); + EXPECT_EQ(tuplets[2]->tick(), Fraction(1, 8)); + EXPECT_EQ(tuplets[2]->staffIdx(), 0); + + const Selection& selection = score->selection(); + EXPECT_TRUE(selection.isRange()); + EXPECT_EQ(selection.tickStart(), Fraction(0, 1)); + EXPECT_EQ(selection.tickEnd(), Fraction(1, 4)); + EXPECT_EQ(selection.staffStart(), 0); + EXPECT_EQ(selection.staffEnd(), 2); + + delete score; +} + +TEST_F(Engraving_TupletTests, addTupletsLinkedStaff) +{ + MasterScore* score = ScoreRW::readScore(TUPLET_DATA_DIR + u"addTuplet.mscx"); + ASSERT_TRUE(score); + addSecondStaff(score, /*linked*/ true); + Measure* m1 = measureAt(score, 0); + + // the first eighth of m1 on both staves: one tuplet, which the linked staff gets too + Segment* first = m1->first(SegmentType::ChordRest); + score->select(first->cr(0), SelectType::RANGE); + score->select(first->cr(VOICES), SelectType::RANGE); + ASSERT_EQ(score->getSelectedChordRests().size(), 2); + EXPECT_EQ(addTuplets(score, Fraction(3, 2)).size(), 1); + EXPECT_TRUE(score->sanityCheck()); + EXPECT_EQ(rhythm(m1), "3:2[16 r16 r16] 8 8 8 8 8 8 8"); + + const Tuplet* linkedTuplet = first->cr(VOICES)->tuplet(); + ASSERT_TRUE(linkedTuplet); + EXPECT_EQ(linkedTuplet->ratio(), Fraction(3, 2)); + EXPECT_FALSE(linkedTuplet->tuplet()); + EXPECT_EQ(linkedTuplet->elements().size(), 3); + EXPECT_TRUE(score->selection().isRange()); + EXPECT_EQ(score->selection().staffEnd(), 2); + + delete score; +} + TEST_F(Engraving_TupletTests, canAddTuplet) { MasterScore* score = ScoreRW::readScore(TUPLET_DATA_DIR + u"addTuplet.mscx"); diff --git a/src/notation/internal/notationinteraction.cpp b/src/notation/internal/notationinteraction.cpp index 578cdb107a69..36d0e15cbba6 100644 --- a/src/notation/internal/notationinteraction.cpp +++ b/src/notation/internal/notationinteraction.cpp @@ -120,6 +120,7 @@ #include "engraving/editing/editstretch.h" #include "engraving/editing/edittie.h" #include "engraving/editing/edittimesig.h" +#include "engraving/editing/edittuplet.h" #include "engraving/editing/editpagelocks.h" #include "engraving/editing/editposition.h" #include "engraving/editing/editsystemlocks.h" @@ -5704,70 +5705,10 @@ void NotationInteraction::addTupletToSelectedChordRests(const TupletOptions& opt return; } - Selection& scoreSelection = score()->selection(); - const bool wasRangeSelection = scoreSelection.isRange(); - const Fraction startTick = scoreSelection.tickStart(); - const Fraction endTick = scoreSelection.tickEnd(); - const staff_idx_t startStaff = scoreSelection.staffStart(); - const staff_idx_t endStaff = scoreSelection.staffEnd(); - Box* startBox = scoreSelection.startBox(); - Box* endBox = scoreSelection.endBox(); - - // Score order, not pointer order - const std::set selectedChordRests = score()->getSelectedChordRests(); - std::vector chordRests(selectedChordRests.begin(), selectedChordRests.end()); - std::sort(chordRests.begin(), chordRests.end(), [](const ChordRest* a, const ChordRest* b) { - return a->tick() != b->tick() ? a->tick() < b->tick() : a->track() < b->track(); + transaction(TranslatableString("undoableAction", "Add tuplet"), [&](mu::engraving::Transaction& tx) { + EditTuplet::addTuplets(tx, score(), options.ratio, options.numberType, options.bracketType, options.autoBaseLen); }); - - startEdit(TranslatableString("undoableAction", "Add tuplet")); - - Tuplet* firstTuplet = nullptr; - - // Make sure each linked group is handled only once - std::unordered_set handledLinks; - - for (ChordRest* chordRest : chordRests) { - if (chordRest->links() && !handledLinks.insert(chordRest->links()).second) { - continue; - } - - if (!chordRest->isGrace() && !(chordRest->isChord() && toChord(chordRest)->isTrillCueNote())) { - Fraction ratio = options.ratio; - if (options.autoBaseLen) { - ratio.setDenominator(Tuplet::computeTupletDenominator(ratio.numerator(), chordRest->ticks())); - } - Tuplet* tuplet = score()->addTuplet(chordRest, ratio, options.numberType, options.bracketType); - if (!firstTuplet) { - firstTuplet = tuplet; - } - } - } - - if (wasRangeSelection) { - scoreSelection.setRangeTicks(startTick, endTick, startStaff, endStaff, startBox, endBox); - scoreSelection.updateSelectedElements(); - } else if (firstTuplet && !firstTuplet->elements().empty()) { - DurationElement* firstElement = firstTuplet->elements().front(); - // SINGLE also moves the input position - if (firstElement->isChord()) { - const std::vector& notes = toChord(firstElement)->notes(); - IF_ASSERT_FAILED(!notes.empty()) { - score()->select(firstElement, SelectType::SINGLE); - } else { - score()->select(notes.front(), SelectType::SINGLE); - score()->select({ notes.begin() + 1, notes.end() }, SelectType::ADD); - } - } else { - score()->select(firstElement, SelectType::SINGLE); - } - } - - if (firstTuplet) { - score()->inputState().setDuration(firstTuplet->baseLen()); - } - - apply(); + notifyAboutNotationChanged(); } void NotationInteraction::addBeamToSelectedChordRests(BeamMode mode)