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 crash disabling quantize #11744

Merged
merged 3 commits into from
Jul 18, 2023
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
46 changes: 29 additions & 17 deletions src/waveform/renderers/waveformmark.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#pragma once

#include <QDomNode>
#include <QImage>

Expand Down Expand Up @@ -92,21 +91,34 @@ class WaveformMark {

typedef QSharedPointer<WaveformMark> WaveformMarkPointer;

inline bool operator<(const WaveformMarkPointer& lhs, const WaveformMarkPointer& rhs) {
double leftPosition = lhs->getSamplePosition();
int leftHotcue = lhs->getHotCue();
double rightPosition = rhs->getSamplePosition();
int rightHotcue = rhs->getHotCue();
if (leftPosition == rightPosition) {
// Sort WaveformMarks without hotcues before those with hotcues;
// if both have hotcues, sort numerically by hotcue number.
if (leftHotcue == Cue::kNoHotCue && rightHotcue != Cue::kNoHotCue) {
return true;
} else if (leftHotcue != Cue::kNoHotCue && rightHotcue == Cue::kNoHotCue) {
return false;
} else {
return leftHotcue < rightHotcue;
// This class provides an immutable sortkey for the WaveformMark using sample
// position and hotcue number. IMPORTANT: The Mark's position may be changed after
// a key's creation, and those updates will not be reflected in these sortkeys.
// Currently they are used to render marks on the Overview, a situation where
// temporarily incorrect sort order is acceptable.
class WaveformMarkSortKey {
daschuer marked this conversation as resolved.
Show resolved Hide resolved
public:
WaveformMarkSortKey(double samplePosition, int hotcue)
daschuer marked this conversation as resolved.
Show resolved Hide resolved
: m_samplePosition(samplePosition),
m_hotcue(hotcue) {
}

bool operator<(const WaveformMarkSortKey& other) const {
if (m_samplePosition == other.m_samplePosition) {
// Sort WaveformMarks without hotcues before those with hotcues;
// if both have hotcues, sort numerically by hotcue number.
if (m_hotcue == Cue::kNoHotCue && other.m_hotcue != Cue::kNoHotCue) {
return true;
} else if (m_hotcue != Cue::kNoHotCue && other.m_hotcue == Cue::kNoHotCue) {
return false;
} else {
return m_hotcue < other.m_hotcue;
}
}
return m_samplePosition < other.m_samplePosition;
}
return leftPosition < rightPosition;
}

private:
double m_samplePosition;
int m_hotcue;
};
38 changes: 22 additions & 16 deletions src/widget/woverview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -427,18 +427,21 @@ void WOverview::updateCues(const QList<CuePointer> &loadedCues) {
pMark->m_text = newLabel;
}
}

m_marksToRender.append(pMark);
}
}

// The loop above only adds WaveformMarks for hotcues to m_marksToRender.
for (const auto& pMark : m_marks) {
if (!m_marksToRender.contains(pMark) && pMark->isValid() && pMark->getSamplePosition() != Cue::kNoPosition && pMark->isVisible()) {
m_marksToRender.append(pMark);
if (pMark->isValid() && pMark->isVisible()) {
double samplePosition = pMark->getSamplePosition();
if (samplePosition != Cue::kNoPosition) {
// Create a stable key for sorting, because the WaveformMark's samplePosition is a
// ControlObject which can change at any time by other threads. Such a change causes
// another updateCues() call, rebuilding m_marksToRender.
auto key = WaveformMarkSortKey(samplePosition, pMark->getHotCue());
m_marksToRender.emplace(key, pMark);
}
}
}
std::sort(m_marksToRender.begin(), m_marksToRender.end());
}

// connecting the tracks cuesUpdated and onMarkChanged is not possible
Expand Down Expand Up @@ -475,8 +478,8 @@ void WOverview::mouseMoveEvent(QMouseEvent* e) {
// the hotcue rendered on top must be assigned to m_pHoveredMark to show
// the CueMenuPopup. To accomplish this, m_marksToRender is iterated in
// reverse and the loop breaks as soon as m_pHoveredMark is set.
for (int i = m_marksToRender.size() - 1; i >= 0; --i) {
WaveformMarkPointer pMark = m_marksToRender.at(i);
for (auto i = m_marksToRender.crbegin(); i != m_marksToRender.crend(); ++i) {
const WaveformMarkPointer& pMark = i->second;
if (pMark->contains(e->pos(), m_orientation)) {
m_pHoveredMark = pMark;
break;
Expand Down Expand Up @@ -826,12 +829,13 @@ void WOverview::drawMarks(QPainter* pPainter, const float offset, const float ga
// the view of labels is not obscured by the playhead.

bool markHovered = false;
for (int i = 0; i < m_marksToRender.size(); ++i) {
WaveformMarkPointer pMark = m_marksToRender.at(i);
PainterScope painterScope(pPainter);

for (auto i = m_marksToRender.cbegin(); i != m_marksToRender.cend(); ++i) {
PainterScope painterScope(pPainter);
const WaveformMarkPointer& pMark = i->second;
double samplePosition = pMark->getSamplePosition();
const float markPosition = math_clamp(
offset + static_cast<float>(m_marksToRender.at(i)->getSamplePosition()) * gain,
offset + static_cast<float>(samplePosition) * gain,
0.0f,
static_cast<float>(width()));
pMark->m_linePosition = markPosition;
Expand Down Expand Up @@ -862,10 +866,11 @@ void WOverview::drawMarks(QPainter* pPainter, const float offset, const float ga
// hovering over it. Elide it if it would render over the next
// label, but do not elide it if the next mark's label is not at the
// same vertical position.
if (pMark != m_pHoveredMark && i < m_marksToRender.size() - 1) {

if (pMark != m_pHoveredMark) {
float nextMarkPosition = -1.0f;
for (int m = i + 1; m < m_marksToRender.size() - 1; ++m) {
WaveformMarkPointer otherMark = m_marksToRender.at(m);
for (auto m = std::next(i); m != m_marksToRender.cend(); ++m) {
const WaveformMarkPointer& otherMark = m->second;
bool otherAtSameHeight = valign == (otherMark->m_align & Qt::AlignVertical_Mask);
// Hotcues always show at least their number.
bool otherHasLabel = !otherMark->m_text.isEmpty() || otherMark->getHotCue() != Cue::kNoHotCue;
Expand Down Expand Up @@ -1116,7 +1121,8 @@ void WOverview::drawMarkLabels(QPainter* pPainter, const float offset, const flo
QFontMetricsF fontMetrics(markerFont);

// Draw WaveformMark labels
for (const auto& pMark : qAsConst(m_marksToRender)) {
for (const auto& pair : m_marksToRender) {
const WaveformMarkPointer& pMark = pair.second;
if (m_pHoveredMark != nullptr && pMark != m_pHoveredMark) {
if (pMark->m_label.intersects(m_pHoveredMark->m_label)) {
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/widget/woverview.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class WOverview : public WWidget, public TrackDropTarget {
// All WaveformMarks
WaveformMarkSet m_marks;
// List of visible WaveformMarks sorted by the order they appear in the track
QList<WaveformMarkPointer> m_marksToRender;
std::map<WaveformMarkSortKey, WaveformMarkPointer> m_marksToRender;
std::vector<WaveformMarkRange> m_markRanges;
WaveformMarkLabel m_cuePositionLabel;
WaveformMarkLabel m_cueTimeDistanceLabel;
Expand Down