Skip to content

[4.6.4] Fix tempo rounding error #30891

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

Merged
merged 1 commit into from
Nov 12, 2025
Merged

[4.6.4] Fix tempo rounding error #30891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 11 additions & 6 deletions src/engraving/dom/score.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ static void markInstrumentsAsPrimary(std::vector<Part*>& parts)
}
}

static BeatsPerSecond roundTempo(const BeatsPerSecond& bps)
{
return muse::RealRound(bps.val, TEMPO_PRECISION);
}

//---------------------------------------------------------
// Score
//---------------------------------------------------------
Expand Down Expand Up @@ -476,7 +481,7 @@ void Score::setUpTempoMap()
int tick2 = tickPositionFrom + pair2.first;

if (tempomap()->find(tick2) == tempomap()->end()) {
tempomap()->setTempo(tick2, BeatsPerSecond(currentBps.val + pair2.second));
tempomap()->setTempo(tick2, roundTempo(currentBps.val + pair2.second));
}
}
}
Expand Down Expand Up @@ -570,7 +575,7 @@ void Score::rebuildTempoAndTimeSigMaps(Measure* measure, std::optional<BeatsPerS
}

if (tt->isNormal() && !tt->isRelative() && !tempoPrimo) {
tempoPrimo = tt->tempo();
tempoPrimo = roundTempo(tt->tempo());
} else if (tt->isRelative()) {
tt->updateRelative();
}
Expand All @@ -583,14 +588,14 @@ void Score::rebuildTempoAndTimeSigMaps(Measure* measure, std::optional<BeatsPerS
} else if (tt->isTempoPrimo() && tt->followText()) {
tempomap()->setTempo(ticks, tempoPrimo ? *tempoPrimo : Constants::DEFAULT_TEMPO);
} else {
tempomap()->setTempo(ticks, tt->tempo());
tempomap()->setTempo(ticks, roundTempo(tt->tempo()));
}
}
}

if (!RealIsNull(stretch) && !RealIsEqual(stretch, 1.0)) {
BeatsPerSecond otempo = tempomap()->tempo(segment.tick().ticks());
BeatsPerSecond ntempo = otempo.val / stretch;
BeatsPerSecond ntempo = roundTempo(otempo.val / stretch);
tempomap()->setTempo(segment.tick().ticks(), ntempo);

Fraction tempoEndTick;
Expand Down Expand Up @@ -655,7 +660,7 @@ void Score::fixAnacrusisTempo(const std::vector<Measure*>& measures) const
Measure* nextMeasure = measure->nextMeasure();
if (nextMeasure) {
if (TempoText* tt = getTempoTextIfExist(nextMeasure); tt) {
tempomap()->setTempo(measure->tick().ticks(), tt->tempo());
tempomap()->setTempo(measure->tick().ticks(), roundTempo(tt->tempo()));
}
}
}
Expand Down Expand Up @@ -4310,7 +4315,7 @@ void Score::setTempo(Segment* segment, BeatsPerSecond tempo)

void Score::setTempo(const Fraction& tick, BeatsPerSecond tempo)
{
tempomap()->setTempo(tick.ticks(), tempo);
tempomap()->setTempo(tick.ticks(), roundTempo(tempo));
setPlaylistDirty();
}

Expand Down
2 changes: 2 additions & 0 deletions src/engraving/dom/tempo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#include "../types/bps.h"

namespace mu::engraving {
static constexpr int TEMPO_PRECISION = 6;

enum class TempoType : char {
INVALID = 0x0, PAUSE = 0x1, FIX = 0x2, RAMP = 0x4
};
Expand Down
3 changes: 2 additions & 1 deletion src/engraving/rw/write/twrite.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
#include "dom/soundflag.h"

#include "dom/tapping.h"
#include "dom/tempo.h"
#include "dom/tempotext.h"
#include "dom/text.h"
#include "dom/textbase.h"
Expand Down Expand Up @@ -3156,7 +3157,7 @@ void TWrite::write(const TempoText* item, XmlWriter& xml, WriteContext& ctx)
{
xml.startElement(item);
writeProperty(item, xml, Pid::PLAY);
xml.tag("tempo", TConv::toXml(item->tempo()));
xml.tag("tempo", TConv::toXml(item->tempo(), TEMPO_PRECISION));
if (item->followText()) {
xml.tag("followText", item->followText());
}
Expand Down
2 changes: 1 addition & 1 deletion ...raving/tests/tempomap_data/absolute_tempo_80_to_120_bpm/absolute_tempo_80_to_120_bpm.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
<sigD>4</sigD>
</TimeSig>
<Tempo>
<tempo>1.33333</tempo>
<tempo>1.333333</tempo>
<followText>1</followText>
<text><sym>metNoteQuarterUp</sym> = 80</text>
</Tempo>
Expand Down
2 changes: 1 addition & 1 deletion src/engraving/tests/tempomap_data/custom_tempo_80_bpm/custom_tempo_80_bpm.mscx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
<sigD>4</sigD>
</TimeSig>
<Tempo>
<tempo>1.33333</tempo>
<tempo>1.333333</tempo>
<followText>1</followText>
<text><sym>metNoteQuarterUp</sym> = 80</text>
</Tempo>
Expand Down
72 changes: 44 additions & 28 deletions src/engraving/tests/tempomap_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ using namespace mu::engraving;

static const String TEMPOMAP_TEST_FILES_DIR("tempomap_data/");

static constexpr double TEMPO_ERROR(0.000001);

class Engraving_TempoMapTests : public ::testing::Test
{
protected:
Expand Down Expand Up @@ -59,8 +61,10 @@ TEST_F(Engraving_TempoMapTests, DEFAULT_TEMPO)

// [THEN] Applied tempo matches our expectations
for (const auto& pair : *tempoMap) {
EXPECT_EQ(pair.second.tempo, expectedTempo);
EXPECT_NEAR(pair.second.tempo.val, expectedTempo.val, TEMPO_ERROR);
}

delete score;
}

/**
Expand All @@ -76,16 +80,18 @@ TEST_F(Engraving_TempoMapTests, ABSOLUTE_TEMPO_80_BPM)
ASSERT_TRUE(score);

// [GIVEN] Expected tempo
BeatsPerSecond expectedTempo = BeatsPerSecond::fromBPM(BeatsPerMinute(80.f));
BeatsPerSecond expectedTempo = BeatsPerSecond::fromBPM(80.0);

// [WHEN] We request score's tempomap it should contain only 1 value, which is our expected tempo
const TempoMap* tempoMap = score->tempomap();
EXPECT_EQ(tempoMap->size(), 1);

// [THEN] Applied tempo matches with our expectations
for (const auto& pair : *tempoMap) {
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(pair.second.tempo.val, 2), muse::RealRound(expectedTempo.val, 2)));
EXPECT_NEAR(pair.second.tempo.val, expectedTempo.val, TEMPO_ERROR);
}

delete score;
}

/**
Expand All @@ -102,9 +108,9 @@ TEST_F(Engraving_TempoMapTests, ABSOLUTE_TEMPO_FROM_80_TO_120_BPM)
ASSERT_TRUE(score);

// [GIVEN] Expected tempomap
std::map<int, BeatsPerSecond> expectedTempoMap = {
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(80.f)) }, // first measure
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(BeatsPerMinute(120.f)) } // 4-th measure
std::map<int, BeatsPerSecond> expectedTempoMap {
{ 0, BeatsPerSecond::fromBPM(80.0) }, // first measure
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(120.0) } // 4-th measure
};

// [WHEN] We request score's tempomap its size matches with our expectations
Expand All @@ -113,8 +119,10 @@ TEST_F(Engraving_TempoMapTests, ABSOLUTE_TEMPO_FROM_80_TO_120_BPM)

// [THEN] Applied tempo matches with our expectations
for (const auto& pair : *tempoMap) {
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(pair.second.tempo.val, 2), muse::RealRound(expectedTempoMap.at(pair.first).val, 2)));
EXPECT_NEAR(pair.second.tempo.val, expectedTempoMap.at(pair.first).val, TEMPO_ERROR);
}

delete score;
}

/**
Expand All @@ -138,9 +146,9 @@ TEST_F(Engraving_TempoMapTests, TEMPO_MULTIPLIER)
tempoMap->setTempoMultiplier(multiplier);

// [GIVEN] Expected tempomap
std::map<int, BeatsPerSecond> expectedTempoMap = {
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(80.0)) }, // first measure
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(BeatsPerMinute(120.0)) } // 4-th measure
std::map<int, BeatsPerSecond> expectedTempoMap {
{ 0, BeatsPerSecond::fromBPM(80.0) }, // first measure
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(120.0) } // 4-th measure
};

// [WHEN] We request score's tempomap its size matches with our expectations
Expand All @@ -150,10 +158,12 @@ TEST_F(Engraving_TempoMapTests, TEMPO_MULTIPLIER)
for (int tick : muse::keys(*tempoMap)) {
double expectedBps = expectedTempoMap[tick].val;

EXPECT_NEAR(tempoMap->at(tick).tempo.val, expectedBps, 0.001);
EXPECT_NEAR(tempoMap->tempo(tick).val, expectedBps, 0.001);
EXPECT_NEAR(tempoMap->multipliedTempo(tick).val, expectedBps * multiplier, 0.001);
EXPECT_NEAR(tempoMap->at(tick).tempo.val, expectedBps, TEMPO_ERROR);
EXPECT_NEAR(tempoMap->tempo(tick).val, expectedBps, TEMPO_ERROR);
EXPECT_NEAR(tempoMap->multipliedTempo(tick).val, expectedBps * multiplier, TEMPO_ERROR);
}

delete score;
}

/**
Expand All @@ -172,8 +182,8 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_ACCELERANDO)

// [GIVEN] Expected tempomap
std::map<int, BeatsPerSecond> expectedTempoMap = {
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(120.f)) }, // beginning of the first measure
{ 6 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(BeatsPerMinute(159.6f)) } // beginning of the last measure
{ 0, BeatsPerSecond::fromBPM(120.0) }, // beginning of the first measure
{ 6 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(159.6) } // beginning of the last measure
};

// [WHEN] We request score's tempomap its size matches with our expectations
Expand All @@ -182,8 +192,10 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_ACCELERANDO)

// [THEN] Applied tempo matches with our expectations
for (const auto& pair : expectedTempoMap) {
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(tempoMap->at(pair.first).tempo.val, 2), muse::RealRound(pair.second.val, 2)));
EXPECT_NEAR(tempoMap->at(pair.first).tempo.val, pair.second.val, TEMPO_ERROR);
}

delete score;
}

/**
Expand All @@ -200,9 +212,9 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_RALLENTANDO)
ASSERT_TRUE(score);

// [GIVEN] Expected tempomap
std::map<int, BeatsPerSecond> expectedTempoMap = {
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(120.f)) }, // beginning of the first measure
{ 6 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(BeatsPerMinute(90.f)) } // beginning of the last measure
std::map<int, BeatsPerSecond> expectedTempoMap {
{ 0, BeatsPerSecond::fromBPM(120.0) }, // beginning of the first measure
{ 6 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(90.0) } // beginning of the last measure
};

// [WHEN] We request score's tempomap its size matches with our expectations
Expand All @@ -211,8 +223,10 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_RALLENTANDO)

// [THEN] Applied tempo matches with our expectations
for (const auto& pair : expectedTempoMap) {
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(tempoMap->at(pair.first).tempo.val, 2), muse::RealRound(pair.second.val, 2)));
EXPECT_NEAR(tempoMap->at(pair.first).tempo.val, pair.second.val, TEMPO_ERROR);
}

delete score;
}

/**
Expand All @@ -235,17 +249,17 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_DOESNT_OVERWRITE_OTHER_TEMP
ASSERT_TRUE(score);

// [GIVEN] Expected tempomap
std::map<int, BeatsPerSecond> expectedTempoMap = {
std::map<int, BeatsPerSecond> expectedTempoMap {
// ritardando (beginning of the first measure)
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(120.f)) },
{ 960, BeatsPerSecond::fromBPM(BeatsPerMinute(112.5f)) },
{ 0, BeatsPerSecond::fromBPM(120.0) },
{ 960, BeatsPerSecond::fromBPM(112.5) },
// tempo marking (80 BPM, the first measure)
{ 1440, BeatsPerSecond::fromBPM(BeatsPerMinute(80.f)) },
{ 1440, BeatsPerSecond::fromBPM(80.0) },
// ritardando (the second measure)
{ 1920, BeatsPerSecond::fromBPM(BeatsPerMinute(105.f)) },
{ 2880, BeatsPerSecond::fromBPM(BeatsPerMinute(97.5f)) },
{ 1920, BeatsPerSecond::fromBPM(105.0) },
{ 2880, BeatsPerSecond::fromBPM(97.5) },
// presto (beginning of the third measure)
{ 3840, BeatsPerSecond::fromBPM(BeatsPerMinute(187.2f)) },
{ 3840, BeatsPerSecond::fromBPM(187.0) },
};

// [WHEN] We request score's tempomap its size matches with our expectations
Expand All @@ -254,6 +268,8 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_DOESNT_OVERWRITE_OTHER_TEMP

// [THEN] Applied tempo matches with our expectations
for (const auto& pair : expectedTempoMap) {
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(tempoMap->at(pair.first).tempo.val, 2), muse::RealRound(pair.second.val, 2)));
EXPECT_NEAR(tempoMap->at(pair.first).tempo.val, pair.second.val, TEMPO_ERROR);
}

delete score;
}
4 changes: 2 additions & 2 deletions src/engraving/types/typesconv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2018,9 +2018,9 @@ AccidentalRole TConv::fromXml(const AsciiStringView& tag, AccidentalRole def)
return ok ? static_cast<AccidentalRole>(r) : def;
}

String TConv::toXml(BeatsPerSecond v)
String TConv::toXml(BeatsPerSecond v, int precision)
{
return String::number(v.val);
return String::number(v.val, precision);
}

BeatsPerSecond TConv::fromXml(const AsciiStringView& tag, BeatsPerSecond def)
Expand Down
2 changes: 1 addition & 1 deletion src/engraving/types/typesconv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class TConv
static String toXml(AccidentalRole v);
static AccidentalRole fromXml(const AsciiStringView& tag, AccidentalRole def);

static String toXml(BeatsPerSecond v);
static String toXml(BeatsPerSecond v, int precision);
static BeatsPerSecond fromXml(const AsciiStringView& tag, BeatsPerSecond def);

static String translatedUserName(DurationType v);
Expand Down
2 changes: 1 addition & 1 deletion src/framework/global/realfn.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ inline double _compare_double_null(COMPARE_DOUBLE_NULL);
inline double _compare_float_epsilon(COMPARE_FLOAT_EPSILON);
inline double _compare_float_null(COMPARE_FLOAT_NULL);

inline int _pow10(int power)
inline constexpr int _pow10(int power)
{
int result = 1;
for (int i = 0; i < power; ++i) {
Expand Down
Loading