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

[WIP] Enable Setting hotcue color via controlobject #1830

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
93c0451
added color column to properties dlg but UI and DB dont update
Oct 2, 2018
e0d73a0
fixed functionality broken by previous commit.
Oct 4, 2018
29df355
woverview waveformsmarks will now reflect their hotcue color
Oct 6, 2018
68efc42
markproperties.m_textColor is now the same color as the mark itself; …
Oct 6, 2018
92d381d
followed daschauers request to VERIFY_OR_DEBUG_ASSERT in a case
Oct 13, 2018
d8057bd
followed uklotzde change requests in widget/woverview
Oct 13, 2018
d4e0735
fixed redundant blank line (codefactor complained)
Oct 14, 2018
2c253ff
fixed inverted VERIFY_OR_DEBUG Statement
Oct 14, 2018
f57e682
Added R/W ControlObject for the HotcueColor
Oct 14, 2018
b0270ba
contrastLineColor of Markers now adapt to MarkerColor Brightness
Oct 18, 2018
9833f67
Removed Comment in cuecontrol.cpp
Oct 19, 2018
d52f107
the marker number color is now set by the skin again, not the m_color
Oct 19, 2018
c8c4150
moved brightness function to color.h; rendermark textColor adapts now
Oct 19, 2018
e58b361
Color is now settable via ComboBox in trackpropertie cuepoints table.
Oct 20, 2018
2372bdf
added more utility functions to util/color.h
Oct 20, 2018
3088c1e
minor changes for performance and codefactor
Oct 21, 2018
20ff8dc
Let skins override cue colors
ferranpujolcamins Dec 2, 2018
1b460cf
Add example, revert before merge.
ferranpujolcamins Dec 3, 2018
ca30b3b
Improve color methods efficiency
ferranpujolcamins Dec 28, 2018
b194d21
Rename Color::defaultRepresentation() to makeDefaultRepresentation()
ferranpujolcamins Dec 28, 2018
54798f3
Let skins override cue colors on overview
ferranpujolcamins Dec 28, 2018
3b324f8
Append "Cue" to the skin node name for waveform cue color overrides
ferranpujolcamins Dec 28, 2018
b6a7dd2
Revert "Add example, revert before merge."
ferranpujolcamins Dec 28, 2018
9afc8d4
Fix WOverview cue colors not being mapped after skin reload
ferranpujolcamins Dec 29, 2018
19480bd
Convert color lists to static const vars
ferranpujolcamins Dec 30, 2018
f950c25
Reduce the number of runtime copies of ColorsRepresentation
ferranpujolcamins Dec 30, 2018
f951a94
Move setupCueColorsRepresentation() to SkinContext
ferranpujolcamins Dec 30, 2018
c49961f
Added JS utility object for dealing with hotcue colors from MixxxControl
Jan 1, 2019
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
26 changes: 26 additions & 0 deletions src/engine/cuecontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@ void CueControl::trackCuesUpdated() {
} else {
// If the old hotcue is the same, then we only need to update
pControl->setPosition(pCue->getPosition());
// NOTE(Swiftb0y): I don't know if this is the smartest location to manage
// please suggest whether this should rather go somewhere else.
pControl->setColor(pCue->getColor());
daschuer marked this conversation as resolved.
Show resolved Hide resolved
}
// Add the hotcue to the list of active hotcues
active_hotcues.insert(hotcue);
Expand Down Expand Up @@ -1018,6 +1021,14 @@ HotcueControl::HotcueControl(QString group, int i)
m_hotcueEnabled = new ControlObject(keyForControl(i, "enabled"));
m_hotcueEnabled->setReadOnly();

// NOTE(Swiftb0y): since ControlObjects only deal with double values,
// the Color is stored as a QRgb (which is just a unsigned int
// representation of the color (AARRGGBB)
m_hotcueColor = new ControlObject(keyForControl(i, "color"));
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
connect(m_hotcueColor, SIGNAL(valueChanged(double)),
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a randomly CO and should not be changed directly by skin or controller scrips, right?
In this case you need to connect the valueChangeRequest() and discard changes there. Programmatically you can use setAndConfirm to bypass the request.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just mimicked how the already existing r/w COs work. I'm trying to plan ahead for integration into skin (where writing to the CO might be useful (maybe, I don't know how such a widget would even be implemented).
And what do you mean by "randomly CO"?

Copy link
Member

Choose a reason for hiding this comment

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

randomly read only

this, SLOT(slotHotcueColorChanged(double)),
Qt::DirectConnection);

m_hotcueSet = new ControlPushButton(keyForControl(i, "set"));
connect(m_hotcueSet, SIGNAL(valueChanged(double)),
this, SLOT(slotHotcueSet(double)),
Expand Down Expand Up @@ -1057,6 +1068,7 @@ HotcueControl::HotcueControl(QString group, int i)
HotcueControl::~HotcueControl() {
delete m_hotcuePosition;
delete m_hotcueEnabled;
delete m_hotcueColor;
delete m_hotcueSet;
delete m_hotcueGoto;
delete m_hotcueGotoAndPlay;
Expand Down Expand Up @@ -1099,6 +1111,11 @@ void HotcueControl::slotHotcuePositionChanged(double newPosition) {
emit(hotcuePositionChanged(this, newPosition));
}

void HotcueControl::slotHotcueColorChanged(double newColor) {
m_pCue->setColor(QColor(static_cast<QRgb>(newColor)));
emit(hotcueColorChanged(this, newColor));
}

double HotcueControl::getPosition() const {
return m_hotcuePosition->get();
}
Expand All @@ -1109,7 +1126,16 @@ void HotcueControl::setCue(CuePointer pCue) {
// because we have a null check for valid data else where in the code
m_pCue = pCue;
}
QColor HotcueControl::getColor() const {
// QRgb is just an unsigned int representation of the
// color components (AARRGGBB) so conversion
// from double shouldn't be an issue
return QColor::fromRgb(static_cast<QRgb>(m_hotcueColor->get()));
}

void HotcueControl::setColor(QColor newColor) {
m_hotcueColor->set(static_cast<double>(newColor.rgb()));
}
void HotcueControl::resetCue() {
// clear pCue first because we have a null check for valid data else where
// in the code
Expand Down
5 changes: 5 additions & 0 deletions src/engine/cuecontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class HotcueControl : public QObject {
void setCue(CuePointer pCue);
void resetCue();
void setPosition(double position);
void setColor(QColor newColor);
QColor getColor() const;

// Used for caching the preview state of this hotcue control.
inline bool isPreviewing() {
Expand All @@ -54,6 +56,7 @@ class HotcueControl : public QObject {
void slotHotcueActivatePreview(double v);
void slotHotcueClear(double v);
void slotHotcuePositionChanged(double newPosition);
void slotHotcueColorChanged(double newColor);

signals:
void hotcueSet(HotcueControl* pHotcue, double v);
Expand All @@ -64,6 +67,7 @@ class HotcueControl : public QObject {
void hotcueActivatePreview(HotcueControl* pHotcue, double v);
void hotcueClear(HotcueControl* pHotcue, double v);
void hotcuePositionChanged(HotcueControl* pHotcue, double newPosition);
void hotcueColorChanged(HotcueControl* pHotcue, double newColor);
void hotcuePlay(double v);

private:
Expand All @@ -76,6 +80,7 @@ class HotcueControl : public QObject {
// Hotcue state controls
ControlObject* m_hotcuePosition;
ControlObject* m_hotcueEnabled;
ControlObject* m_hotcueColor;
// Hotcue button controls
ControlObject* m_hotcueSet;
ControlObject* m_hotcueGoto;
Expand Down
22 changes: 19 additions & 3 deletions src/library/dlgtrackinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,15 @@ void DlgTrackInfo::populateCues(TrackPointer pTrack) {
// Make the duration read only
durationItem->setFlags(Qt::NoItemFlags);

QColor cueColor = pCue->getColor();

m_cueMap[row] = pCue;
cueTable->insertRow(row);
cueTable->setItem(row, 0, new QTableWidgetItem(rowStr));
cueTable->setItem(row, 1, durationItem);
cueTable->setItem(row, 2, new QTableWidgetItem(hotcue));
cueTable->setItem(row, 3, new QTableWidgetItem(pCue->getLabel()));
cueTable->setItem(row, 3, new QTableWidgetItem(cueColor.name().toUpper()));
cueTable->setItem(row, 4, new QTableWidgetItem(pCue->getLabel()));
row += 1;
}
cueTable->setSortingEnabled(true);
Expand Down Expand Up @@ -381,10 +384,13 @@ void DlgTrackInfo::saveTrack() {
for (int row = 0; row < cueTable->rowCount(); ++row) {
QTableWidgetItem* rowItem = cueTable->item(row, 0);
QTableWidgetItem* hotcueItem = cueTable->item(row, 2);
QTableWidgetItem* labelItem = cueTable->item(row, 3);
QTableWidgetItem* colorItem = cueTable->item(row, 3);
QTableWidgetItem* labelItem = cueTable->item(row, 4);

if (!rowItem || !hotcueItem || !labelItem)
VERIFY_OR_DEBUG_ASSERT(rowItem && hotcueItem && colorItem && labelItem) {
qWarning() << "unable to retrieve cells from cueTable row";
continue;
}

int oldRow = rowItem->data(Qt::DisplayRole).toInt();
CuePointer pCue(m_cueMap.value(oldRow, CuePointer()));
Expand All @@ -404,6 +410,16 @@ void DlgTrackInfo::saveTrack() {
pCue->setHotCue(-1);
}

QVariant vHotCueColor = colorItem->data(Qt::DisplayRole);
if (vHotcue.canConvert<QString>()) {
QString colorString = vHotCueColor.toString();
auto color = QColor(colorString);
if (color.isValid()) {
pCue->setColor(color);
}
// do nothing for now.
}

QString label = labelItem->data(Qt::DisplayRole).toString();
pCue->setLabel(label);
}
Expand Down
5 changes: 5 additions & 0 deletions src/library/dlgtrackinfo.ui
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,11 @@ Often results in higher quality beatgrids, but will not do well on tracks that h
<string>Hotcue</string>
</property>
</column>
<column>
<property name="text">
<string>Color</string>
</property>
</column>
<column>
<property name="text">
<string>Label</string>
Expand Down
1 change: 1 addition & 0 deletions src/waveform/renderers/waveformmarkproperties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ WaveformMarkProperties::WaveformMarkProperties(const QDomNode& node,
const WaveformSignalColors& signalColors,
int hotCue) {
m_color = context.selectString(node, "Color");
// TODO (Swiftb0y): get CuePointer and color for m_color instead of skin color.
if (!m_color.isValid()) {
// As a fallback, grab the color from the parent's AxesColor
m_color = signalColors.getAxesColor();
Expand Down
4 changes: 3 additions & 1 deletion src/waveform/renderers/waveformrendermark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ void WaveformRenderMark::generateMarkImage(WaveformMark* pMark) {
// Prepare colors for drawing of marker lines
QColor lineColor = markProperties.m_color;
lineColor.setAlpha(200);
QColor contrastLineColor(0,0,0,120);
QColor contrastLineColor = (brightness(lineColor) < 130) ?
QColor(255,255,255,120) :
QColor(0,0,0,120);

// Draw marker lines
if (m_waveformRenderer->getOrientation() == Qt::Horizontal) {
Expand Down
11 changes: 10 additions & 1 deletion src/waveform/renderers/waveformrendermark.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "skin/skincontext.h"
#include "util/class.h"
#include "util/math.h"
#include "waveform/renderers/waveformmarkset.h"
#include "waveform/renderers/waveformrendererabstract.h"
#include "library/dao/cue.h"
Expand All @@ -31,7 +32,15 @@ class WaveformRenderMark : public QObject, public WaveformRendererAbstract {

private:
void generateMarkImage(WaveformMark* pMark);

// algorithm by http://www.nbdtech.com/Blog/archive/2008/04/27/Calculating-the-Perceived-Brightness-of-a-Color.aspx
// NOTE(Swiftb0y): please suggest if I should you use other methods
// (like the W3C algorithm) or if this approach is to to performance hungry
int brightness(QColor& c) {
return static_cast<int>(sqrtf(
c.red() * c.red() * .241 +
c.green() * c.green() * .691 +
c.blue() * c.blue() * .068));
};
WaveformMarkSet m_marks;
DISALLOW_COPY_AND_ASSIGN(WaveformRenderMark);
};
Expand Down
27 changes: 27 additions & 0 deletions src/widget/woverview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ void WOverview::slotLoadingTrack(TrackPointer pNewTrack, TrackPointer pOldTrack)
this, SLOT(slotWaveformSummaryUpdated()));
connect(pNewTrack.get(), SIGNAL(analyzerProgress(int)),
this, SLOT(slotAnalyzerProgress(int)));
connect(pNewTrack.get(), SIGNAL(cuesUpdated()),
this, SLOT(receiveCuesUpdated()));

slotAnalyzerProgress(pNewTrack->getAnalyzerProgress());
} else {
Expand All @@ -247,6 +249,7 @@ void WOverview::onEndOfTrackChange(double v) {

void WOverview::onMarkChanged(double /*v*/) {
//qDebug() << "WOverview::onMarkChanged()" << v;
updateCues(m_pCurrentTrack->getCuePoints());
update();
}

Expand All @@ -255,6 +258,30 @@ void WOverview::onMarkRangeChange(double /*v*/) {
update();
}

// currently only updates the mark color but it could be easily extended.
void WOverview::updateCues(const QList<CuePointer> &loadedCues) {
for (CuePointer currentCue: loadedCues) {
const WaveformMarkPointer currentMark = m_marks.getHotCueMark(currentCue->getHotCue());

if (currentMark && currentMark->isValid()) {
WaveformMarkProperties markProperties = currentMark->getProperties();
const QColor newColor = currentCue->getColor();

if (newColor != markProperties.m_color || newColor != markProperties.m_textColor) {
markProperties.m_color = newColor;
markProperties.m_textColor = newColor;
currentMark->setProperties(markProperties);
}
}
}
}

// connecting the tracks cuesUpdated and onMarkChanged is not possible
// due to the incompatible signatures. This is a "wrapper" workaround
void WOverview::receiveCuesUpdated() {
onMarkChanged(0);
}

void WOverview::mouseMoveEvent(QMouseEvent* e) {
if (m_orientation == Qt::Horizontal) {
m_iPos = math_clamp(e->x(), 0, width() - 1);
Expand Down
3 changes: 3 additions & 0 deletions src/widget/woverview.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class WOverview : public WWidget {

void onMarkChanged(double v);
void onMarkRangeChange(double v);
void receiveCuesUpdated();

void slotWaveformSummaryUpdated();
void slotAnalyzerProgress(int progress);
Expand All @@ -99,6 +100,8 @@ class WOverview : public WWidget {
return (static_cast<double>(position) + m_b) / m_a;
}

void updateCues(const QList<CuePointer> &loadedCues);

const QString m_group;
UserSettingsPointer m_pConfig;
ControlProxy* m_endOfTrackControl;
Expand Down