Skip to content
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.

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

[MU4] Fix GH#8893: allow for 128th as the denominator for measures, prevent crash on shorter ones #8894

Merged
merged 4 commits into from
Oct 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/appshell/view/preferences/importpreferencesmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ QVariantList ImportPreferencesModel::shortestNotes() const
QVariantMap { { "title", qtrc("appshell", "32nd") }, { "value", division() / 8 } },
QVariantMap { { "title", qtrc("appshell", "64th") }, { "value", division() / 16 } },
QVariantMap { { "title", qtrc("appshell", "128th") }, { "value", division() / 32 } },
QVariantMap { { "title", qtrc("appshell", "256h") }, { "value", division() / 64 } },
QVariantMap { { "title", qtrc("appshell", "256th") }, { "value", division() / 64 } },
QVariantMap { { "title", qtrc("appshell", "512th") }, { "value", division() / 128 } },
QVariantMap { { "title", qtrc("appshell", "1024th") }, { "value", division() / 256 } }
};
Expand Down
7 changes: 7 additions & 0 deletions src/engraving/libmscore/edit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,13 @@ TextBase* Score::addText(Tid type)
text = "<sym>metNote64thUp</sym> = 80";
}
break;
case 128:
if (f.numerator() % 3 == 0) {
text = "<sym>metNote64ndUp</sym><sym>space</sym><sym>metAugmentationDot</sym> = 80";
} else {
text = "<sym>metNote128thUp</sym> = 80";
}
break;
default:
break;
}
Expand Down
5 changes: 4 additions & 1 deletion src/engraving/libmscore/mscore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,11 @@ std::vector<MScoreError> MScore::errorList {
{ MsError::CANNOT_SPLIT_MEASURE_FIRST_BEAT, "m1", QT_TRANSLATE_NOOP("error", "Cannot split measure here:\n" "First beat of measure") },
{ MsError::CANNOT_SPLIT_MEASURE_TUPLET, "m2", QT_TRANSLATE_NOOP("error", "Cannot split measure here:\n" "Cannot split tuplet") },
{ MsError::INSUFFICIENT_MEASURES, "m3", QT_TRANSLATE_NOOP("error",
"Measure repeat cannot be added here:\nInsufficient or unequal measures") },
"Measure repeat cannot be added here:\n"
"Insufficient or unequal measures") },
{ MsError::CANNOT_SPLIT_MEASURE_REPEAT, "m4", QT_TRANSLATE_NOOP("error", "Cannot split measure repeat") },
{ MsError::CANNOT_SPLIT_MEASURE_TOO_SHORT, "m5",
QT_TRANSLATE_NOOP("error", "Cannot split measure here:\n" "Measure would be too short") },

{ MsError::CANNOT_REMOVE_TIME_TUPLET, "d1", QT_TRANSLATE_NOOP("error",
"Cannot remove time from tuplet:\nPlease select the complete tuplet and retry") },
Expand Down
1 change: 1 addition & 0 deletions src/engraving/libmscore/mscore.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ enum class MsError {
CANNOT_SPLIT_MEASURE_TUPLET,
INSUFFICIENT_MEASURES,
CANNOT_SPLIT_MEASURE_REPEAT,
CANNOT_SPLIT_MEASURE_TOO_SHORT,
CANNOT_REMOVE_TIME_TUPLET,
CANNOT_REMOVE_TIME_MEASURE_REPEAT,
NO_DEST,
Expand Down
7 changes: 5 additions & 2 deletions src/engraving/libmscore/splitMeasure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ void Score::cmdSplitMeasure(ChordRest* cr)

//---------------------------------------------------------
// splitMeasure
// return true on success
//---------------------------------------------------------

void Score::splitMeasure(Segment* segment)
Expand Down Expand Up @@ -133,7 +132,7 @@ void Score::splitMeasure(Segment* segment)
if (ticks1.denominator() < measure->ticks().denominator()) {
if (measure->ticks().denominator() % m1->timesig().denominator() == 0) {
int mult = measure->ticks().denominator() / ticks1.denominator();
// *= operator audomatically reduces via GCD, so rather do literal multiplication:
// *= operator automatically reduces via GCD, so rather do literal multiplication:
ticks1.setDenominator(ticks1.denominator() * mult);
ticks1.setNumerator(ticks1.numerator() * mult);
}
Expand All @@ -145,6 +144,10 @@ void Score::splitMeasure(Segment* segment)
ticks2.setNumerator(ticks2.numerator() * mult);
}
}
if (ticks1.denominator() > 128 || ticks2.denominator() > 128) {
MScore::setError(MsError::CANNOT_SPLIT_MEASURE_TOO_SHORT);
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Difficult to test, because of #8895. It doesn't crash, it doesn't split the measure, but also doesn't give any error message

m1->adjustToLen(ticks1, false);
m2->adjustToLen(ticks2, false);
range.write(this, m1->tick());
Expand Down
2 changes: 1 addition & 1 deletion src/engraving/rw/measurerw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ void MeasureRW::readMeasure(Measure* measure, XmlReader& e, ReadContext& ctx, in
qDebug("illegal measure size <%s>", qPrintable(e.attribute("len")));
}
irregular = true;
if (measure->_len.numerator() <= 0 || measure->_len.denominator() <= 0) {
if (measure->_len.numerator() <= 0 || measure->_len.denominator() <= 0 || measure->_len.denominator() > 128) {
e.raiseError(QObject::tr("MSCX error at line %1: invalid measure length: %2").arg(e.lineNumber()).arg(measure->_len.toString()));
return;
}
Expand Down
16 changes: 8 additions & 8 deletions src/importexport/musicxml/internal/musicxml/importmxmlpass1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2220,27 +2220,27 @@ void MusicXMLParserPass1::measure(const QString& partId,
mDura = Fraction(4, 4);
}

// if necessary, round up to an integral number of 1/64s,
// if necessary, round up to an integral number of 1/128th,
// to comply with MuseScores actual measure length constraints
Fraction length = mDura * Fraction(64, 1);
Fraction length = mDura * Fraction(128, 1);
Fraction correctedLength = mDura;
length.reduce();
if (length.denominator() != 1) {
Fraction roundDown = Fraction(length.numerator() / length.denominator(), 64);
Fraction roundUp = Fraction(length.numerator() / length.denominator() + 1, 64);
// mDura is not an integer multiple of 1/64;
// first check if the duration is larger than an integer multiple of 1/64
Fraction roundDown = Fraction(length.numerator() / length.denominator(), 128);
Fraction roundUp = Fraction(length.numerator() / length.denominator() + 1, 128);
// mDura is not an integer multiple of 1/128;
// first check if the duration is larger than an integer multiple of 1/128
// by an amount smaller than the minimum division resolution
// in that case, round down (rounding errors have possibly occurred),
// otherwise, round up
if ((_divs > 0) && ((mDura - roundDown) < Fraction(1, 4 * _divs))) {
_logger->logError(QString("rounding down measure duration %1 to %2")
.arg(qPrintable(mDura.print())).arg(qPrintable(roundDown.print())),
.arg(qPrintable(mDura.print()), qPrintable(roundDown.print())),
&_e);
correctedLength = roundDown;
} else {
_logger->logError(QString("rounding up measure duration %1 to %2")
.arg(qPrintable(mDura.print())).arg(qPrintable(roundUp.print())),
.arg(qPrintable(mDura.print()), qPrintable(roundUp.print())),
&_e);
correctedLength = roundUp;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ static void fillGap(Measure* measure, int track, const Fraction& tstart, const F
// qDebug("\nfillGIFV fillGap(measure %p track %d tstart %d tend %d) restLen %d len",
// measure, track, tstart, tend, restLen);
// note: as MScore::division (#ticks in a quarter note) equals 480
// MScore::division / 64 (#ticks in a 256th note) uequals 7.5 but is rounded down to 7
// MScore::division / 64 (#ticks in a 256th note) equals 7.5 but is rounded down to 7
while (restLen > Fraction(1, 256)) {
Fraction len = restLen;
TDuration d(TDuration::DurationType::V_INVALID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,6 @@
<type>128th</type>
<stem>up</stem>
</note>
<note print-object="no">
<rest/>
<duration>1</duration>
<voice>1</voice>
<type>128th</type>
</note>
</measure>
<measure number="3">
<note>
Expand Down
9 changes: 9 additions & 0 deletions src/notation/internal/masternotation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,15 @@ mu::Ret MasterNotation::setupNewScore(Ms::MasterScore* score, Ms::MasterScore* t
bpm *= 16;
}
break;
case 128:
if (ts.numerator() % 3 == 0) {
text = "<sym>metNote64ndUp</sym><sym>space</sym><sym>metAugmentationDot</sym> = %1";
bpm *= 6;
} else {
text = "<sym>metNote128thUp</sym> = %1";
bpm *= 16;
}
break;
default:
break;
}
Expand Down
5 changes: 5 additions & 0 deletions src/notation/view/widgets/measureproperties.ui
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@
<string>64</string>
</property>
</item>
<item>
<property name="text">
<string>128</string>
</property>
</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this going to be replaced with a qml version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, it isn't yet though.

</widget>
</item>
<item row="0" column="4" colspan="2">
Expand Down
4 changes: 4 additions & 0 deletions src/palette/view/widgets/timedialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ int TimeDialog::denominator2Idx(int denominator) const
break;
case 64: val = 6;
break;
case 128: val = 7;
break;
}
return val;
}
Expand All @@ -197,6 +199,8 @@ int TimeDialog::denominator() const
break;
case 6: val = 64;
break;
case 7: val = 128;
break;
}
return val;
}
Expand Down
5 changes: 5 additions & 0 deletions src/palette/view/widgets/timedialog.ui
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@
<string>64</string>
</property>
</item>
<item>
<property name="text">
<string>128</string>
</property>
</item>
</widget>
</item>
</layout>
Expand Down
2 changes: 1 addition & 1 deletion src/project/view/additionalinfomodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ QVariantMap AdditionalInfoModel::measureCountRange() const

QVariantList AdditionalInfoModel::timeSignatureDenominators() const
{
return QVariantList { 1, 2, 4, 8, 16, 32, 64 };
return QVariantList { 1, 2, 4, 8, 16, 32, 64, 128 };
}

void AdditionalInfoModel::setKeySignature(QVariantMap keySignature)
Expand Down