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

Add command for clearing waveform to track's context menu #1197

Merged
merged 5 commits into from
Mar 30, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 20 additions & 5 deletions src/widget/woverview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,27 @@ void WOverview::slotWaveformSummaryUpdated() {
return;
}
m_pWaveform = pTrack->getWaveformSummary();
// If the waveform is already complete, just draw it.
if (m_pWaveform && m_pWaveform->getCompletion() == m_pWaveform->getDataSize()) {
m_actualCompletion = 0;
if (drawNextPixmapPart()) {
update();
if (m_pWaveform) {
// If the waveform is already complete, just draw it.
if (m_pWaveform->getCompletion() == m_pWaveform->getDataSize()) {
m_actualCompletion = 0;
if (drawNextPixmapPart()) {
update();
}
}
} else {
// Null waveform pointer means waveform was cleared.
if (m_pWaveformSourceImage) {
delete m_pWaveformSourceImage;
Copy link
Member

Choose a reason for hiding this comment

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

The ownership of m_pWaveformSourceImage is unclear. This is not your fault, but we should fix it here, because this PR builds on top of this situation.
I think the best solution is to move m_pWaveformSourceImage into the inherited classes and put it into a std::unique_ptr
this WOverview should have a abstract virtual void invalidateImage() = 0, that is then implemented in the Inherited classes to actually clear the Image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have decided to take a (slightly) different approach with this and make this QImage statically allocated (like m_waveformImageScaled is). I hope you don't mind. 😃

m_pWaveformSourceImage = nullptr;
}

m_dAnalyzerProgress = 1.0;
m_actualCompletion = 0;
m_waveformPeak = -1.0;
m_pixmapDone = false;

update();
}
}

Expand Down
25 changes: 25 additions & 0 deletions src/widget/wtracktableview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,10 @@ void WTrackTableView::createActions() {
connect(m_pClearBeatsAction, SIGNAL(triggered()),
this, SLOT(slotClearBeats()));

m_pClearWaveformAction = new QAction(tr("Clear Waveform"), this);
connect(m_pClearWaveformAction, SIGNAL(triggered()),
this, SLOT(slotClearWaveform()));

m_pReplayGainResetAction = new QAction(tr("Reset ReplayGain"), this);
connect(m_pReplayGainResetAction, SIGNAL(triggered()),
this, SLOT(slotReplayGainReset()));
Expand Down Expand Up @@ -929,6 +933,8 @@ void WTrackTableView::contextMenuEvent(QContextMenuEvent* event) {
m_pBPMMenu->addAction(m_pClearBeatsAction);
}

m_pMenu->addAction(m_pClearWaveformAction);

m_pMenu->addAction(m_pReplayGainResetAction);

m_pMenu->addSeparator();
Expand Down Expand Up @@ -1606,6 +1612,25 @@ void WTrackTableView::slotClearBeats() {
}
}

void WTrackTableView::slotClearWaveform() {
TrackModel* trackModel = getTrackModel();
if (trackModel == nullptr) {
return;
}

AnalysisDao& analysisDao = m_pTrackCollection->getAnalysisDAO();
QModelIndexList indices = selectionModel()->selectedRows();
for (const QModelIndex& index : indices) {
TrackPointer pTrack = trackModel->getTrack(index);
if (!pTrack) {
continue;
}
analysisDao.deleteAnalysesForTrack(pTrack->getId());
Copy link
Member

Choose a reason for hiding this comment

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

Will this delete the all analysis data?
I think we should rethink the use-cases for "ClearWaveform" and name this feature accordingly.
IMHO clearing all analysis makes sense if the use-case is, that the track samples have changed.
In this case all position related data is invalid.
What will happen with cue points?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhm Well, by looking at the AnalysisDao code, AnalysisDao is used just for storing waveforms, so this indeed is only clearing track's waveforms.

pTrack->setWaveform(WaveformPointer());
pTrack->setWaveformSummary(WaveformPointer());
}
}

void WTrackTableView::slotReplayGainReset() {
QModelIndexList indices = selectionModel()->selectedRows();
TrackModel* trackModel = getTrackModel();
Expand Down
4 changes: 4 additions & 0 deletions src/widget/wtracktableview.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class WTrackTableView : public WLibraryTableView {
void slotUnlockBpm();
void slotScaleBpm(int);
void slotClearBeats();
void slotClearWaveform();
void slotReplayGainReset();
// Signalled 20 times per second (every 50ms) by GuiTick.
void slotGuiTick50ms(double);
Expand Down Expand Up @@ -162,6 +163,9 @@ class WTrackTableView : public WLibraryTableView {
// Clear track beats
QAction* m_pClearBeatsAction;

// Clear track waveform
QAction* m_pClearWaveformAction;

// Replay Gain feature
QAction *m_pReplayGainResetAction;

Expand Down
18 changes: 16 additions & 2 deletions src/widget/wwaveformviewer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,25 @@ void WWaveformViewer::slotTrackLoaded(TrackPointer track) {
}

void WWaveformViewer::slotLoadingTrack(TrackPointer pNewTrack, TrackPointer pOldTrack) {
Q_UNUSED(pNewTrack);
Q_UNUSED(pOldTrack);
if (pOldTrack) {
disconnect(pOldTrack.get(), SIGNAL(waveformUpdated()),
this, SLOT(slotWaveformUpdated()));
}

if (m_waveformWidget) {
m_waveformWidget->setTrack(TrackPointer());
}

if (pNewTrack) {
connect(pNewTrack.get(), SIGNAL(waveformUpdated()),
this, SLOT(slotWaveformUpdated()));
Copy link
Member

Choose a reason for hiding this comment

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

the track was just cleared above. Can we move this to slotTrackLoaded, to keep symmetry?

}
}

void WWaveformViewer::slotWaveformUpdated() {
if (m_waveformWidget) {
m_waveformWidget->setTrack(m_waveformWidget->getTrackInfo());
Copy link
Member

Choose a reason for hiding this comment

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

What dos this line? Can we replace it by a member function with a nice name?

}
}

void WWaveformViewer::onZoomChange(double zoom) {
Expand Down
2 changes: 2 additions & 0 deletions src/widget/wwaveformviewer.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ private slots:
m_waveformWidget = nullptr;
}

void slotWaveformUpdated();

private:
void setWaveformWidget(WaveformWidgetAbstract* waveformWidget);
WaveformWidgetAbstract* getWaveformWidget() {
Expand Down