From fe950002d5e863fecc77b05af4f5ae2ab48c8376 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Wed, 24 Apr 2024 21:15:32 +0200 Subject: [PATCH 1/5] Only set sample clips for valid files Check if a non-empty buffer was loaded and only set the sample clip if that's the case. Fix code formatting. --- src/gui/clips/SampleClipView.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/gui/clips/SampleClipView.cpp b/src/gui/clips/SampleClipView.cpp index ef46325e4e6..78058051290 100644 --- a/src/gui/clips/SampleClipView.cpp +++ b/src/gui/clips/SampleClipView.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include "GuiApplication.h" @@ -183,16 +184,26 @@ void SampleClipView::mouseDoubleClickEvent( QMouseEvent * ) { QString af = SampleLoader::openAudioFile(); - if ( af.isEmpty() ) {} //Don't do anything if no file is loaded - else if (af == m_clip->m_sample.sampleFile()) - { //Instead of reloading the existing file, just reset the size + // Don't do anything if no file is loaded + if (af.isEmpty()) { return; } + + if (af == m_clip->m_sample.sampleFile()) + { + // Don't reload the same file again. Just reset the size. int length = static_cast(m_clip->m_sample.sampleSize() / Engine::framesPerTick()); m_clip->changeLength(length); } else - { //Otherwise load the new file as ususal - m_clip->setSampleFile( af ); - Engine::getSong()->setModified(); + { + // Load the new file and set it for the clip + auto sampleBuffer = SampleLoader::createBufferFromFile(af); + if (sampleBuffer != SampleBuffer::emptyBuffer()) + { + m_clip->setSampleBuffer(sampleBuffer); + + // TODO Why do we have to do this here? This should happen in the core... + Engine::getSong()->setModified(); + } } } From 14ce3cc0c988181b6af62f186737b96982d9fc06 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sat, 27 Apr 2024 16:48:01 +0200 Subject: [PATCH 2/5] Set "song modified" flag in core Move setting the song to modified towards the core in the context of `SampleClip`. Previously the `SampleClipView` did this but it's none of it's business. Introduce `SampleClip::changeLengthToSampleLength` which changes the length of the clip to the length of the sample. This was also previously done by the view which is again the wrong place to do the necessary calculations. --- include/SampleClip.h | 1 + src/core/SampleClip.cpp | 9 +++++++++ src/gui/clips/SampleClipView.cpp | 7 +------ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/include/SampleClip.h b/include/SampleClip.h index da11996b14c..c2e8247b539 100644 --- a/include/SampleClip.h +++ b/include/SampleClip.h @@ -55,6 +55,7 @@ class SampleClip : public Clip SampleClip& operator=( const SampleClip& that ) = delete; void changeLength( const TimePos & _length ) override; + void changeLengthToSampleLength(); const QString& sampleFile() const; void saveSettings( QDomDocument & _doc, QDomElement & _parent ) override; diff --git a/src/core/SampleClip.cpp b/src/core/SampleClip.cpp index 42d4f644168..e43f8682c39 100644 --- a/src/core/SampleClip.cpp +++ b/src/core/SampleClip.cpp @@ -112,6 +112,11 @@ void SampleClip::changeLength( const TimePos & _length ) Clip::changeLength(std::max(static_cast(_length), 1)); } +void SampleClip::changeLengthToSampleLength() +{ + int length = static_cast(m_sample.sampleSize() / Engine::framesPerTick()); + changeLength(length); +} @@ -129,6 +134,8 @@ void SampleClip::setSampleBuffer(std::shared_ptr sb) updateLength(); emit sampleChanged(); + + Engine::getSong()->setModified(); } void SampleClip::setSampleFile(const QString& sf) @@ -210,6 +217,8 @@ void SampleClip::setIsPlaying(bool isPlaying) void SampleClip::updateLength() { emit sampleChanged(); + + Engine::getSong()->setModified(); } diff --git a/src/gui/clips/SampleClipView.cpp b/src/gui/clips/SampleClipView.cpp index 78058051290..7c30c904ca6 100644 --- a/src/gui/clips/SampleClipView.cpp +++ b/src/gui/clips/SampleClipView.cpp @@ -128,7 +128,6 @@ void SampleClipView::dropEvent( QDropEvent * _de ) m_clip->updateLength(); update(); _de->accept(); - Engine::getSong()->setModified(); } else { @@ -190,8 +189,7 @@ void SampleClipView::mouseDoubleClickEvent( QMouseEvent * ) if (af == m_clip->m_sample.sampleFile()) { // Don't reload the same file again. Just reset the size. - int length = static_cast(m_clip->m_sample.sampleSize() / Engine::framesPerTick()); - m_clip->changeLength(length); + m_clip->changeLengthToSampleLength(); } else { @@ -200,9 +198,6 @@ void SampleClipView::mouseDoubleClickEvent( QMouseEvent * ) if (sampleBuffer != SampleBuffer::emptyBuffer()) { m_clip->setSampleBuffer(sampleBuffer); - - // TODO Why do we have to do this here? This should happen in the core... - Engine::getSong()->setModified(); } } } From cb9f18e7384ed912221da15ef403c4934d358ecb Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sat, 27 Apr 2024 16:52:20 +0200 Subject: [PATCH 3/5] Remove QMessageBox include Remove the `QMessageBox` include which is a left over of another fix attempt. --- src/gui/clips/SampleClipView.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/gui/clips/SampleClipView.cpp b/src/gui/clips/SampleClipView.cpp index 7c30c904ca6..024de734af5 100644 --- a/src/gui/clips/SampleClipView.cpp +++ b/src/gui/clips/SampleClipView.cpp @@ -26,7 +26,6 @@ #include #include -#include #include #include "GuiApplication.h" From ef681e1b79ada8292e1a4f331bcd40baa6598661 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Sun, 28 Apr 2024 20:42:22 +0200 Subject: [PATCH 4/5] Remove static_cast Remove an unnecessary `static_cast`. --- src/core/SampleClip.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/SampleClip.cpp b/src/core/SampleClip.cpp index e43f8682c39..ddbb24f2c30 100644 --- a/src/core/SampleClip.cpp +++ b/src/core/SampleClip.cpp @@ -114,7 +114,7 @@ void SampleClip::changeLength( const TimePos & _length ) void SampleClip::changeLengthToSampleLength() { - int length = static_cast(m_sample.sampleSize() / Engine::framesPerTick()); + int length = m_sample.sampleSize() / Engine::framesPerTick(); changeLength(length); } From 4d5bf7c60783a6eefa9231b1b298d418f44c5513 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Mon, 29 Apr 2024 17:20:26 +0200 Subject: [PATCH 5/5] Add SampleClip::hasSampleFileLoaded Add the method `SampleClip::hasSampleFileLoaded` and remove some comments. --- include/SampleClip.h | 1 + src/core/SampleClip.cpp | 5 +++++ src/gui/clips/SampleClipView.cpp | 11 ++++------- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/include/SampleClip.h b/include/SampleClip.h index c2e8247b539..3beca338bcd 100644 --- a/include/SampleClip.h +++ b/include/SampleClip.h @@ -57,6 +57,7 @@ class SampleClip : public Clip void changeLength( const TimePos & _length ) override; void changeLengthToSampleLength(); const QString& sampleFile() const; + bool hasSampleFileLoaded(const QString & filename) const; void saveSettings( QDomDocument & _doc, QDomElement & _parent ) override; void loadSettings( const QDomElement & _this ) override; diff --git a/src/core/SampleClip.cpp b/src/core/SampleClip.cpp index ddbb24f2c30..9a1c0731a97 100644 --- a/src/core/SampleClip.cpp +++ b/src/core/SampleClip.cpp @@ -125,6 +125,11 @@ const QString& SampleClip::sampleFile() const return m_sample.sampleFile(); } +bool SampleClip::hasSampleFileLoaded(const QString & filename) const +{ + return m_sample.sampleFile() == filename; +} + void SampleClip::setSampleBuffer(std::shared_ptr sb) { { diff --git a/src/gui/clips/SampleClipView.cpp b/src/gui/clips/SampleClipView.cpp index 024de734af5..30f09caa860 100644 --- a/src/gui/clips/SampleClipView.cpp +++ b/src/gui/clips/SampleClipView.cpp @@ -180,20 +180,17 @@ void SampleClipView::mouseReleaseEvent(QMouseEvent *_me) void SampleClipView::mouseDoubleClickEvent( QMouseEvent * ) { - QString af = SampleLoader::openAudioFile(); + const QString selectedAudioFile = SampleLoader::openAudioFile(); - // Don't do anything if no file is loaded - if (af.isEmpty()) { return; } + if (selectedAudioFile.isEmpty()) { return; } - if (af == m_clip->m_sample.sampleFile()) + if (m_clip->hasSampleFileLoaded(selectedAudioFile)) { - // Don't reload the same file again. Just reset the size. m_clip->changeLengthToSampleLength(); } else { - // Load the new file and set it for the clip - auto sampleBuffer = SampleLoader::createBufferFromFile(af); + auto sampleBuffer = SampleLoader::createBufferFromFile(selectedAudioFile); if (sampleBuffer != SampleBuffer::emptyBuffer()) { m_clip->setSampleBuffer(sampleBuffer);