Skip to content

Commit 6584fe0

Browse files
RomanPudashkinRomanPudashkin
committed
Round tempo to ensure consistent values between editing and reopening scores
Small float-point differences in tempo caused slight shifts in event timing between the in-memory and saved score. Although these differences were usually insignificant, they became important when using online sounds, since cached data depended on precise event timings. As a result, reopening the score could trigger unnecessary reprocessing
1 parent 6b7d9ee commit 6584fe0

File tree

9 files changed

+65
-41
lines changed

9 files changed

+65
-41
lines changed

‎src/engraving/dom/score.cpp‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,11 @@ static void markInstrumentsAsPrimary(std::vector<Part*>& parts)
144144
}
145145
}
146146

147+
static BeatsPerSecond roundTempo(const BeatsPerSecond& bps)
148+
{
149+
return muse::RealRound(bps.val, TEMPO_PRECISION);
150+
}
151+
147152
//---------------------------------------------------------
148153
// Score
149154
//---------------------------------------------------------
@@ -476,7 +481,7 @@ void Score::setUpTempoMap()
476481
int tick2 = tickPositionFrom + pair2.first;
477482

478483
if (tempomap()->find(tick2) == tempomap()->end()) {
479-
tempomap()->setTempo(tick2, BeatsPerSecond(currentBps.val + pair2.second));
484+
tempomap()->setTempo(tick2, roundTempo(currentBps.val + pair2.second));
480485
}
481486
}
482487
}
@@ -570,7 +575,7 @@ void Score::rebuildTempoAndTimeSigMaps(Measure* measure, std::optional<BeatsPerS
570575
}
571576

572577
if (tt->isNormal() && !tt->isRelative() && !tempoPrimo) {
573-
tempoPrimo = tt->tempo();
578+
tempoPrimo = roundTempo(tt->tempo());
574579
} else if (tt->isRelative()) {
575580
tt->updateRelative();
576581
}
@@ -583,14 +588,14 @@ void Score::rebuildTempoAndTimeSigMaps(Measure* measure, std::optional<BeatsPerS
583588
} else if (tt->isTempoPrimo() && tt->followText()) {
584589
tempomap()->setTempo(ticks, tempoPrimo ? *tempoPrimo : Constants::DEFAULT_TEMPO);
585590
} else {
586-
tempomap()->setTempo(ticks, tt->tempo());
591+
tempomap()->setTempo(ticks, roundTempo(tt->tempo()));
587592
}
588593
}
589594
}
590595

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

596601
Fraction tempoEndTick;
@@ -655,7 +660,7 @@ void Score::fixAnacrusisTempo(const std::vector<Measure*>& measures) const
655660
Measure* nextMeasure = measure->nextMeasure();
656661
if (nextMeasure) {
657662
if (TempoText* tt = getTempoTextIfExist(nextMeasure); tt) {
658-
tempomap()->setTempo(measure->tick().ticks(), tt->tempo());
663+
tempomap()->setTempo(measure->tick().ticks(), roundTempo(tt->tempo()));
659664
}
660665
}
661666
}
@@ -4310,7 +4315,7 @@ void Score::setTempo(Segment* segment, BeatsPerSecond tempo)
43104315

43114316
void Score::setTempo(const Fraction& tick, BeatsPerSecond tempo)
43124317
{
4313-
tempomap()->setTempo(tick.ticks(), tempo);
4318+
tempomap()->setTempo(tick.ticks(), roundTempo(tempo));
43144319
setPlaylistDirty();
43154320
}
43164321

‎src/engraving/dom/tempo.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
#include "../types/bps.h"
3131

3232
namespace mu::engraving {
33+
static constexpr int TEMPO_PRECISION = 6;
34+
3335
enum class TempoType : char {
3436
INVALID = 0x0, PAUSE = 0x1, FIX = 0x2, RAMP = 0x4
3537
};

‎src/engraving/rw/write/twrite.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@
142142
#include "dom/soundflag.h"
143143

144144
#include "dom/tapping.h"
145+
#include "dom/tempo.h"
145146
#include "dom/tempotext.h"
146147
#include "dom/text.h"
147148
#include "dom/textbase.h"
@@ -3156,7 +3157,7 @@ void TWrite::write(const TempoText* item, XmlWriter& xml, WriteContext& ctx)
31563157
{
31573158
xml.startElement(item);
31583159
writeProperty(item, xml, Pid::PLAY);
3159-
xml.tag("tempo", TConv::toXml(item->tempo()));
3160+
xml.tag("tempo", TConv::toXml(item->tempo(), TEMPO_PRECISION));
31603161
if (item->followText()) {
31613162
xml.tag("followText", item->followText());
31623163
}

‎src/engraving/tests/tempomap_data/absolute_tempo_80_to_120_bpm/absolute_tempo_80_to_120_bpm.mscx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@
125125
<sigD>4</sigD>
126126
</TimeSig>
127127
<Tempo>
128-
<tempo>1.33333</tempo>
128+
<tempo>1.333333</tempo>
129129
<followText>1</followText>
130130
<text><sym>metNoteQuarterUp</sym> = 80</text>
131131
</Tempo>

‎src/engraving/tests/tempomap_data/custom_tempo_80_bpm/custom_tempo_80_bpm.mscx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@
125125
<sigD>4</sigD>
126126
</TimeSig>
127127
<Tempo>
128-
<tempo>1.33333</tempo>
128+
<tempo>1.333333</tempo>
129129
<followText>1</followText>
130130
<text><sym>metNoteQuarterUp</sym> = 80</text>
131131
</Tempo>

‎src/engraving/tests/tempomap_tests.cpp‎

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ using namespace mu::engraving;
3232

3333
static const String TEMPOMAP_TEST_FILES_DIR("tempomap_data/");
3434

35+
static constexpr double TEMPO_ERROR(0.000001);
36+
3537
class Engraving_TempoMapTests : public ::testing::Test
3638
{
3739
protected:
@@ -59,8 +61,10 @@ TEST_F(Engraving_TempoMapTests, DEFAULT_TEMPO)
5961

6062
// [THEN] Applied tempo matches our expectations
6163
for (const auto& pair : *tempoMap) {
62-
EXPECT_EQ(pair.second.tempo, expectedTempo);
64+
EXPECT_NEAR(pair.second.tempo.val, expectedTempo.val, TEMPO_ERROR);
6365
}
66+
67+
delete score;
6468
}
6569

6670
/**
@@ -76,16 +80,18 @@ TEST_F(Engraving_TempoMapTests, ABSOLUTE_TEMPO_80_BPM)
7680
ASSERT_TRUE(score);
7781

7882
// [GIVEN] Expected tempo
79-
BeatsPerSecond expectedTempo = BeatsPerSecond::fromBPM(BeatsPerMinute(80.f));
83+
BeatsPerSecond expectedTempo = BeatsPerSecond::fromBPM(80.0);
8084

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

8589
// [THEN] Applied tempo matches with our expectations
8690
for (const auto& pair : *tempoMap) {
87-
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(pair.second.tempo.val, 2), muse::RealRound(expectedTempo.val, 2)));
91+
EXPECT_NEAR(pair.second.tempo.val, expectedTempo.val, TEMPO_ERROR);
8892
}
93+
94+
delete score;
8995
}
9096

9197
/**
@@ -102,9 +108,9 @@ TEST_F(Engraving_TempoMapTests, ABSOLUTE_TEMPO_FROM_80_TO_120_BPM)
102108
ASSERT_TRUE(score);
103109

104110
// [GIVEN] Expected tempomap
105-
std::map<int, BeatsPerSecond> expectedTempoMap = {
106-
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(80.f)) }, // first measure
107-
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(BeatsPerMinute(120.f)) } // 4-th measure
111+
std::map<int, BeatsPerSecond> expectedTempoMap {
112+
{ 0, BeatsPerSecond::fromBPM(80.0) }, // first measure
113+
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(120.0) } // 4-th measure
108114
};
109115

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

114120
// [THEN] Applied tempo matches with our expectations
115121
for (const auto& pair : *tempoMap) {
116-
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(pair.second.tempo.val, 2), muse::RealRound(expectedTempoMap.at(pair.first).val, 2)));
122+
EXPECT_NEAR(pair.second.tempo.val, expectedTempoMap.at(pair.first).val, TEMPO_ERROR);
117123
}
124+
125+
delete score;
118126
}
119127

120128
/**
@@ -138,9 +146,9 @@ TEST_F(Engraving_TempoMapTests, TEMPO_MULTIPLIER)
138146
tempoMap->setTempoMultiplier(multiplier);
139147

140148
// [GIVEN] Expected tempomap
141-
std::map<int, BeatsPerSecond> expectedTempoMap = {
142-
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(80.0)) }, // first measure
143-
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(BeatsPerMinute(120.0)) } // 4-th measure
149+
std::map<int, BeatsPerSecond> expectedTempoMap {
150+
{ 0, BeatsPerSecond::fromBPM(80.0) }, // first measure
151+
{ 4 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(120.0) } // 4-th measure
144152
};
145153

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

153-
EXPECT_NEAR(tempoMap->at(tick).tempo.val, expectedBps, 0.001);
154-
EXPECT_NEAR(tempoMap->tempo(tick).val, expectedBps, 0.001);
155-
EXPECT_NEAR(tempoMap->multipliedTempo(tick).val, expectedBps * multiplier, 0.001);
161+
EXPECT_NEAR(tempoMap->at(tick).tempo.val, expectedBps, TEMPO_ERROR);
162+
EXPECT_NEAR(tempoMap->tempo(tick).val, expectedBps, TEMPO_ERROR);
163+
EXPECT_NEAR(tempoMap->multipliedTempo(tick).val, expectedBps * multiplier, TEMPO_ERROR);
156164
}
165+
166+
delete score;
157167
}
158168

159169
/**
@@ -172,8 +182,8 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_ACCELERANDO)
172182

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

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

183193
// [THEN] Applied tempo matches with our expectations
184194
for (const auto& pair : expectedTempoMap) {
185-
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(tempoMap->at(pair.first).tempo.val, 2), muse::RealRound(pair.second.val, 2)));
195+
EXPECT_NEAR(tempoMap->at(pair.first).tempo.val, pair.second.val, TEMPO_ERROR);
186196
}
197+
198+
delete score;
187199
}
188200

189201
/**
@@ -200,9 +212,9 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_RALLENTANDO)
200212
ASSERT_TRUE(score);
201213

202214
// [GIVEN] Expected tempomap
203-
std::map<int, BeatsPerSecond> expectedTempoMap = {
204-
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(120.f)) }, // beginning of the first measure
205-
{ 6 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(BeatsPerMinute(90.f)) } // beginning of the last measure
215+
std::map<int, BeatsPerSecond> expectedTempoMap {
216+
{ 0, BeatsPerSecond::fromBPM(120.0) }, // beginning of the first measure
217+
{ 6 * 4 * Constants::DIVISION, BeatsPerSecond::fromBPM(90.0) } // beginning of the last measure
206218
};
207219

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

212224
// [THEN] Applied tempo matches with our expectations
213225
for (const auto& pair : expectedTempoMap) {
214-
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(tempoMap->at(pair.first).tempo.val, 2), muse::RealRound(pair.second.val, 2)));
226+
EXPECT_NEAR(tempoMap->at(pair.first).tempo.val, pair.second.val, TEMPO_ERROR);
215227
}
228+
229+
delete score;
216230
}
217231

218232
/**
@@ -235,17 +249,17 @@ TEST_F(Engraving_TempoMapTests, GRADUAL_TEMPO_CHANGE_DOESNT_OVERWRITE_OTHER_TEMP
235249
ASSERT_TRUE(score);
236250

237251
// [GIVEN] Expected tempomap
238-
std::map<int, BeatsPerSecond> expectedTempoMap = {
252+
std::map<int, BeatsPerSecond> expectedTempoMap {
239253
// ritardando (beginning of the first measure)
240-
{ 0, BeatsPerSecond::fromBPM(BeatsPerMinute(120.f)) },
241-
{ 960, BeatsPerSecond::fromBPM(BeatsPerMinute(112.5f)) },
254+
{ 0, BeatsPerSecond::fromBPM(120.0) },
255+
{ 960, BeatsPerSecond::fromBPM(112.5) },
242256
// tempo marking (80 BPM, the first measure)
243-
{ 1440, BeatsPerSecond::fromBPM(BeatsPerMinute(80.f)) },
257+
{ 1440, BeatsPerSecond::fromBPM(80.0) },
244258
// ritardando (the second measure)
245-
{ 1920, BeatsPerSecond::fromBPM(BeatsPerMinute(105.f)) },
246-
{ 2880, BeatsPerSecond::fromBPM(BeatsPerMinute(97.5f)) },
259+
{ 1920, BeatsPerSecond::fromBPM(105.0) },
260+
{ 2880, BeatsPerSecond::fromBPM(97.5) },
247261
// presto (beginning of the third measure)
248-
{ 3840, BeatsPerSecond::fromBPM(BeatsPerMinute(187.2f)) },
262+
{ 3840, BeatsPerSecond::fromBPM(187.0) },
249263
};
250264

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

255269
// [THEN] Applied tempo matches with our expectations
256270
for (const auto& pair : expectedTempoMap) {
257-
EXPECT_TRUE(muse::RealIsEqual(muse::RealRound(tempoMap->at(pair.first).tempo.val, 2), muse::RealRound(pair.second.val, 2)));
271+
EXPECT_NEAR(tempoMap->at(pair.first).tempo.val, pair.second.val, TEMPO_ERROR);
258272
}
273+
274+
delete score;
259275
}

‎src/engraving/types/typesconv.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2018,9 +2018,9 @@ AccidentalRole TConv::fromXml(const AsciiStringView& tag, AccidentalRole def)
20182018
return ok ? static_cast<AccidentalRole>(r) : def;
20192019
}
20202020

2021-
String TConv::toXml(BeatsPerSecond v)
2021+
String TConv::toXml(BeatsPerSecond v, int precision)
20222022
{
2023-
return String::number(v.val);
2023+
return String::number(v.val, precision);
20242024
}
20252025

20262026
BeatsPerSecond TConv::fromXml(const AsciiStringView& tag, BeatsPerSecond def)

‎src/engraving/types/typesconv.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class TConv
119119
static String toXml(AccidentalRole v);
120120
static AccidentalRole fromXml(const AsciiStringView& tag, AccidentalRole def);
121121

122-
static String toXml(BeatsPerSecond v);
122+
static String toXml(BeatsPerSecond v, int precision);
123123
static BeatsPerSecond fromXml(const AsciiStringView& tag, BeatsPerSecond def);
124124

125125
static String translatedUserName(DurationType v);

‎src/framework/global/realfn.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ inline double _compare_double_null(COMPARE_DOUBLE_NULL);
3939
inline double _compare_float_epsilon(COMPARE_FLOAT_EPSILON);
4040
inline double _compare_float_null(COMPARE_FLOAT_NULL);
4141

42-
inline int _pow10(int power)
42+
inline constexpr int _pow10(int power)
4343
{
4444
int result = 1;
4545
for (int i = 0; i < power; ++i) {

0 commit comments

Comments
 (0)