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

Fix #16519: L/R Arrow During Playback Moves From Playhead Current Location #19038

Merged
merged 3 commits into from
Aug 15, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/notation/inotationplayback.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class INotationPlayback

virtual const Tempo& tempo(muse::midi::tick_t tick) const = 0;
virtual MeasureBeat beat(muse::midi::tick_t tick) const = 0;
virtual muse::midi::tick_t beatToTick(int measureIndex, int beatIndex) const = 0;
virtual muse::midi::tick_t beatToRawTick(int measureIndex, int beatIndex) const = 0;

virtual double tempoMultiplier() const = 0;
virtual void setTempoMultiplier(double multiplier) = 0;
Expand Down
34 changes: 34 additions & 0 deletions src/notation/internal/notationactioncontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "engraving/dom/note.h"
#include "engraving/dom/text.h"
#include "engraving/dom/sig.h"

#include "translation.h"
#include "log.h"
Expand Down Expand Up @@ -990,6 +991,39 @@ void NotationActionController::move(MoveDirection direction, bool quickly)
break;
case MoveDirection::Right:
case MoveDirection::Left:
if (playbackController()->isPlaying()) {
MeasureBeat beat = playbackController()->currentBeat();
int targetBeatIdx = beat.beatIndex;
int targetMeasureIdx = beat.measureIndex;
int increment = (direction == MoveDirection::Right ? 1 : -1);

if (quickly) {
targetBeatIdx = 0;
targetMeasureIdx += increment;
if (targetMeasureIdx > beat.maxMeasureIndex) {
targetMeasureIdx = beat.maxMeasureIndex;
} else if (targetMeasureIdx < 0) {
targetMeasureIdx = 0;
}
} else {
targetBeatIdx += increment;
if (targetBeatIdx > beat.maxBeatIndex) {
targetBeatIdx = 0;
targetMeasureIdx += 1;
} else if (targetBeatIdx < 0) {
targetMeasureIdx -= 1;

// Set target beat to max beat of previous bar
engraving::TimeSigMap* timeSigMap = currentMasterNotation()->masterScore()->sigmap();
int targetBarStartTick = timeSigMap->bar2tick(targetMeasureIdx, 0);
targetBeatIdx = timeSigMap->timesig(Fraction::fromTicks(targetBarStartTick)).timesig().numerator() - 1;
Comment on lines +1016 to +1019
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the changes I made + #include "engraving/dom/sig.h" at the top (I thought it might be unclear because I rebased the branch)

Copy link
Contributor

Choose a reason for hiding this comment

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

@RomanPudashkin do you think we should try to extract these lines into a method that can be reused here and in NotationPlayback::beat? Or is that not worth the effort?
(The question would also be, where to put such a method.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Cosmetic improvements can be made later

}
}

playbackController()->seekBeat(targetMeasureIdx, targetBeatIdx);
return;
}

if (interaction->isTextEditingStarted() && textNavigationAvailable()) {
navigateToTextElementInNearMeasure(direction);
return;
Expand Down
2 changes: 1 addition & 1 deletion src/notation/internal/notationplayback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ MeasureBeat NotationPlayback::beat(tick_t tick) const
return measureBeat;
}

tick_t NotationPlayback::beatToTick(int measureIndex, int beatIndex) const
tick_t NotationPlayback::beatToRawTick(int measureIndex, int beatIndex) const
{
return score() ? score()->sigmap()->bar2tick(measureIndex, beatIndex) : 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/notation/internal/notationplayback.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class NotationPlayback : public INotationPlayback, public muse::async::Asyncable

const Tempo& tempo(muse::midi::tick_t tick) const override;
MeasureBeat beat(muse::midi::tick_t tick) const override;
muse::midi::tick_t beatToTick(int measureIndex, int beatIndex) const override;
muse::midi::tick_t beatToRawTick(int measureIndex, int beatIndex) const override;

double tempoMultiplier() const override;
void setTempoMultiplier(double multiplier) override;
Expand Down
43 changes: 28 additions & 15 deletions src/playback/internal/playbackcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,18 @@ void PlaybackController::reset()
stop();
}

void PlaybackController::seek(const midi::tick_t tick)
void PlaybackController::seekRawTick(const midi::tick_t tick)
{
if (m_currentTick == tick) {
return;
}

seek(tickToSecs(tick));
RetVal<midi::tick_t> playedTick = notationPlayback()->playPositionTickByRawTick(tick);
if (!playedTick.ret) {
return;
}

seek(playedTickToSecs(playedTick.val));
}

void PlaybackController::seek(const audio::secs_t secs)
Expand Down Expand Up @@ -363,7 +368,13 @@ void PlaybackController::seekElement(const notation::EngravingItem* element)
return;
}

seek(tick.val);
seek(playedTickToSecs(tick.val));
}

void PlaybackController::seekBeat(int measureIndex, int beatIndex)
{
secs_t targetSecs = beatToSecs(measureIndex, beatIndex);
seek(targetSecs);
}

void PlaybackController::seekListSelection()
Expand All @@ -384,12 +395,7 @@ void PlaybackController::seekRangeSelection()

midi::tick_t startTick = selectionRange()->startTick().ticks();

RetVal<midi::tick_t> tick = notationPlayback()->playPositionTickByRawTick(startTick);
if (!tick.ret) {
return;
}

seek(tick.val);
seekRawTick(startTick);
}

void PlaybackController::onAudioResourceChanged(const InstrumentTrackId& trackId,
Expand Down Expand Up @@ -637,7 +643,7 @@ secs_t PlaybackController::playbackStartSecs() const
if (!startTick.ret) {
return 0;
}
return tickToSecs(startTick.val);
return playedTickToSecs(startTick.val);
}

return 0;
Expand Down Expand Up @@ -799,8 +805,8 @@ void PlaybackController::updateLoop()
return;
}

secs_t fromSecs = tickToSecs(playbackTickFrom.val);
secs_t toSecs = tickToSecs(playbackTickTo.val);
secs_t fromSecs = playedTickToSecs(playbackTickFrom.val);
secs_t toSecs = playedTickToSecs(playbackTickTo.val);
currentPlayer()->setLoop(secsToMilisecs(fromSecs), secsToMilisecs(toSecs));

enableLoop();
Expand Down Expand Up @@ -1434,7 +1440,14 @@ MeasureBeat PlaybackController::currentBeat() const

secs_t PlaybackController::beatToSecs(int measureIndex, int beatIndex) const
{
return notationPlayback() ? tickToSecs(notationPlayback()->beatToTick(measureIndex, beatIndex)) : secs_t { 0 };
if (!notationPlayback()) {
return 0;
}

muse::midi::tick_t rawTick = notationPlayback()->beatToRawTick(measureIndex, beatIndex);
muse::midi::tick_t playedTick = notationPlayback()->playPositionTickByRawTick(rawTick).val;

return playedTickToSecs(playedTick);
}

double PlaybackController::tempoMultiplier() const
Expand All @@ -1456,7 +1469,7 @@ void PlaybackController::setTempoMultiplier(double multiplier)
}

notationPlayback()->setTempoMultiplier(multiplier);
seek(tick);
seekRawTick(tick);
updateLoop();

if (playing) {
Expand Down Expand Up @@ -1576,7 +1589,7 @@ bool PlaybackController::canReceiveAction(const ActionCode&) const
return m_masterNotation != nullptr && m_masterNotation->hasParts();
}

muse::audio::secs_t PlaybackController::tickToSecs(int tick) const
muse::audio::secs_t PlaybackController::playedTickToSecs(int tick) const
{
return secs_t(notationPlayback()->playedTickToSec(tick));
}
6 changes: 3 additions & 3 deletions src/playback/internal/playbackcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class PlaybackController : public IPlaybackController, public muse::actions::Act
void playElements(const std::vector<const notation::EngravingItem*>& elements) override;
void playMetronome(int tick) override;
void seekElement(const notation::EngravingItem* element) override;
void seekBeat(int measureIndex, int beatIndex) override;

bool actionChecked(const muse::actions::ActionCode& actionCode) const override;
muse::async::Channel<muse::actions::ActionCode> actionCheckedChanged() const override;
Expand All @@ -111,7 +112,6 @@ class PlaybackController : public IPlaybackController, public muse::actions::Act
void setIsExportingAudio(bool exporting) override;

bool canReceiveAction(const muse::actions::ActionCode& code) const override;

private:
muse::audio::IPlayerPtr currentPlayer() const;

Expand All @@ -125,7 +125,7 @@ class PlaybackController : public IPlaybackController, public muse::actions::Act

void updateCurrentTempo();

void seek(const muse::midi::tick_t tick);
void seekRawTick(const muse::midi::tick_t tick);
void seek(const muse::audio::secs_t secs);

bool isPaused() const;
Expand Down Expand Up @@ -202,7 +202,7 @@ class PlaybackController : public IPlaybackController, public muse::actions::Act
void removeNonExistingTracks();
void removeTrack(const engraving::InstrumentTrackId& instrumentTrackId);

muse::audio::secs_t tickToSecs(int tick) const;
muse::audio::secs_t playedTickToSecs(int tick) const;

notation::INotationPtr m_notation;
notation::IMasterNotationPtr m_masterNotation;
Expand Down
1 change: 1 addition & 0 deletions src/playback/iplaybackcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class IPlaybackController : MODULE_EXPORT_INTERFACE
virtual void playElements(const std::vector<const notation::EngravingItem*>& elements) = 0;
virtual void playMetronome(int tick) = 0;
virtual void seekElement(const notation::EngravingItem* element) = 0;
virtual void seekBeat(int measureIndex, int beatIndex) = 0;

virtual bool actionChecked(const muse::actions::ActionCode& actionCode) const = 0;
virtual muse::async::Channel<muse::actions::ActionCode> actionCheckedChanged() const = 0;
Expand Down
1 change: 1 addition & 0 deletions src/playback/tests/mocks/playbackcontrollermock.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class PlaybackControllerMock : public IPlaybackController
MOCK_METHOD(void, playElements, ((const std::vector<const notation::EngravingItem*>&)), (override));
MOCK_METHOD(void, playMetronome, (int), (override));
MOCK_METHOD(void, seekElement, (const notation::EngravingItem*), (override));
MOCK_METHOD(void, seekBeat, (int, int), (override));

MOCK_METHOD(bool, actionChecked, (const muse::actions::ActionCode&), (const, override));
MOCK_METHOD(muse::async::Channel<muse::actions::ActionCode>, actionCheckedChanged, (), (const, override));
Expand Down
4 changes: 4 additions & 0 deletions src/stubs/playback/playbackcontrollerstub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ void PlaybackControllerStub::seekElement(const notation::EngravingItem*)
{
}

void PlaybackControllerStub::seekBeat(int, int)
{
}

bool PlaybackControllerStub::actionChecked(const ActionCode&) const
{
return false;
Expand Down
1 change: 1 addition & 0 deletions src/stubs/playback/playbackcontrollerstub.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class PlaybackControllerStub : public IPlaybackController
void playElements(const std::vector<const notation::EngravingItem*>& elements) override;
void playMetronome(int tick) override;
void seekElement(const notation::EngravingItem* element) override;
void seekBeat(int measureIndex, int beatIndex) override;

bool actionChecked(const muse::actions::ActionCode& actionCode) const override;
muse::async::Channel<muse::actions::ActionCode> actionCheckedChanged() const override;
Expand Down