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

Cue colors #992

Merged
merged 57 commits into from
Aug 27, 2016
Merged
Show file tree
Hide file tree
Changes from 53 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
1aeb8b7
Modify Cue for cue point colors.
ferranpujolcamins Aug 14, 2016
0d67283
Modify database shcema to support cue colors.
ferranpujolcamins Aug 14, 2016
b7e864d
Modify Cue constructor to get cue color.
ferranpujolcamins Aug 14, 2016
f2d0cfa
Implement Cue::getColor() and Cue::setColor(...)
ferranpujolcamins Aug 14, 2016
b78cd5d
Modify CueDAO to support cue colors
ferranpujolcamins Aug 14, 2016
ac14bde
Update requiredSchemVersion
ferranpujolcamins Aug 14, 2016
1065e48
Tabs for spaces
ferranpujolcamins Aug 14, 2016
c68086b
Hotcues now display their label in the waveform.
ferranpujolcamins Jan 16, 2015
fbcc455
Make waveformmarks be rendered with the color stored in Cue
ferranpujolcamins Aug 14, 2016
07997b7
Use QColor for defaultColor constant and enclose it in a namespace.
ferranpujolcamins Aug 17, 2016
ac06fc1
Explicitly check for uninitialized mark values in slotCuesUpdated()
ferranpujolcamins Aug 17, 2016
48eed12
Improve coding style
ferranpujolcamins Aug 17, 2016
168859b
Add debug asserts for valid parameter to getHotCueMark
ferranpujolcamins Aug 17, 2016
9895b87
Enclose iMaxCueLabelLenght in a namespace
ferranpujolcamins Aug 17, 2016
5b1dfea
Refactor Cues enumeration with a for each loop in slotCuesUpdated
ferranpujolcamins Aug 17, 2016
a0229f7
Fix empty string check in WaveformRenderMark
ferranpujolcamins Aug 17, 2016
fb939de
Change cue color database type to integer (QRgb)
ferranpujolcamins Aug 17, 2016
e977942
Rename constants
ferranpujolcamins Aug 17, 2016
c1aec44
Change references for pointers.
ferranpujolcamins Aug 18, 2016
9f7230d
Replace WaveformMark CO normal pointer by a unique_ptr
ferranpujolcamins Aug 18, 2016
683638d
Make WaveformMarkSet store mark pointer instead of marks
ferranpujolcamins Aug 18, 2016
e4b585c
Fix make_unique template inference
ferranpujolcamins Aug 18, 2016
4503e06
Fix make_unique template inference
ferranpujolcamins Aug 18, 2016
3edd680
Fix WaveformMarkSet access to WaveformMarks pointers
ferranpujolcamins Aug 18, 2016
ae5b59e
Make m_defaultMark a pointer
ferranpujolcamins Aug 18, 2016
e0bfdfd
Convert m_defaultMark to a unique_ptr
ferranpujolcamins Aug 18, 2016
bd49637
Rename m_pointCos to m_pPointCos
ferranpujolcamins Aug 18, 2016
22b0587
Typo
ferranpujolcamins Aug 18, 2016
67414df
Fix getDefaultMark
ferranpujolcamins Aug 18, 2016
4e24c09
Add clone constructor to WaveformMark
ferranpujolcamins Aug 18, 2016
deba2ae
Provide proper setter for hot cue marks in WaveformMarkSet
ferranpujolcamins Aug 18, 2016
1f1b107
Fix last hotcue not rendered
ferranpujolcamins Aug 19, 2016
cf6bfc4
Fix wrong hotcue number rendered
ferranpujolcamins Aug 19, 2016
7be5597
Remove unused getDefaultMark() from WaveformMarkSet
ferranpujolcamins Aug 19, 2016
58405ee
Add WaveformMarkPreferences class
ferranpujolcamins Aug 21, 2016
30dd1bb
Simplify WaveformMark and make it use WaveformMarkProperties
ferranpujolcamins Aug 21, 2016
da6b9d9
Adapt WaveformMarkSet to WaveformMarkProperties
ferranpujolcamins Aug 21, 2016
db9b553
Adapt renderer and overview to WaveformMarkProperties
ferranpujolcamins Aug 21, 2016
2f8863b
Fix cue mark label rectangle too small for long label
ferranpujolcamins Aug 21, 2016
1d8ef77
Rename WaveformMarkSet::m_pDefaultMark to m_defaultMark
ferranpujolcamins Aug 21, 2016
74bb3bc
Remove explicit WaveformMark constructor
ferranpujolcamins Aug 21, 2016
f3f92c2
Rename WaveformMark:properties() to getProperties()
ferranpujolcamins Aug 21, 2016
f05d1b2
Improve WaveformMarkSet::setup code
ferranpujolcamins Aug 21, 2016
3c4f6da
Make WaveformProperties final.
ferranpujolcamins Aug 21, 2016
360467f
Move hotcue property to WaveformMark
ferranpujolcamins Aug 21, 2016
09a501a
Move setter/getter definition to .h
ferranpujolcamins Aug 21, 2016
a89470c
Add hotCue constructor to Waveformmark
ferranpujolcamins Aug 21, 2016
4ef4cae
Remove redundant type conv. and move operator[] impl. to.h
ferranpujolcamins Aug 21, 2016
6eeea37
Fix missing include
ferranpujolcamins Aug 21, 2016
2859e50
Make WaveformMarkSet getter methods const.
ferranpujolcamins Aug 21, 2016
030c904
Replace QSharedPointer::data() by a const ref
ferranpujolcamins Aug 21, 2016
7c7dea9
Improve WaveformMarkSet::setup code
ferranpujolcamins Aug 21, 2016
b682f11
Introduce WaveformmarkPointer
ferranpujolcamins Aug 21, 2016
6f5770f
Change parameter naming
ferranpujolcamins Aug 21, 2016
10e5ef8
Rewrite cue default color in database schema to decimal
ferranpujolcamins Aug 21, 2016
5b2fb04
Add comment for cue color in database scheme
ferranpujolcamins Aug 21, 2016
fd9b4a5
Merge branch 'master' into CueColors
ferranpujolcamins Aug 27, 2016
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
1 change: 1 addition & 0 deletions build/depends.py
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,7 @@ def sources(self, build):

"waveform/renderers/waveformrenderersignalbase.cpp",
"waveform/renderers/waveformmark.cpp",
"waveform/renderers/waveformmarkproperties.cpp",
"waveform/renderers/waveformmarkset.cpp",
"waveform/renderers/waveformmarkrange.cpp",
"waveform/renderers/glwaveformrenderersimplesignal.cpp",
Expand Down
9 changes: 9 additions & 0 deletions res/schema.xml
Original file line number Diff line number Diff line change
Expand Up @@ -416,4 +416,13 @@ METADATA
ALTER TABLE library ADD COLUMN tracktotal TEXT DEFAULT '//';
</sql>
</revision>
<revision version="27" min_compatible="3">
<description>
Add cue color support. Default color is #FF0000.
See library/dao/cue.h.
</description>
<sql>
ALTER TABLE cues ADD COLUMN color INTEGER DEFAULT 0xFFFF0000 NOT NULL;
</sql>
</revision>
</schema>
25 changes: 22 additions & 3 deletions src/library/dao/cue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
#include "library/dao/cue.h"
#include "util/assert.h"

namespace {
const QColor kDefaultColor = QColor("#FF0000");
}

Cue::~Cue() {
qDebug() << "~Cue()" << m_iId;
}
Expand All @@ -19,20 +23,22 @@ Cue::Cue(TrackId trackId)
m_iPosition(-1),
m_iLength(0),
m_iHotCue(-1),
m_label("") {
m_label(),
m_color(kDefaultColor) {
}


Cue::Cue(int id, TrackId trackId, Cue::CueType type, int position, int length,
int hotCue, QString label)
int hotCue, QString label, QColor color)
: m_bDirty(false),
m_iId(id),
m_trackId(trackId),
m_type(type),
m_iPosition(position),
m_iLength(length),
m_iHotCue(hotCue),
m_label(label) {
m_label(label),
m_color(color) {
}

int Cue::getId() const {
Expand Down Expand Up @@ -134,6 +140,19 @@ void Cue::setLabel(const QString label) {
emit(updated());
}

QColor Cue::getColor() const {
QMutexLocker lock(&m_mutex);
return m_color;
}

void Cue::setColor(const QColor color) {
QMutexLocker lock(&m_mutex);
m_color = color;
m_bDirty = true;
lock.unlock();
emit(updated());
}

bool Cue::isDirty() const {
QMutexLocker lock(&m_mutex);
return m_bDirty;
Expand Down
9 changes: 7 additions & 2 deletions src/library/dao/cue.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <QObject>
#include <QMutex>
#include <QSharedPointer>
#include <QColor>

#include "track/trackid.h"

Expand Down Expand Up @@ -44,15 +45,18 @@ class Cue : public QObject {
void setHotCue(int hotCue);

QString getLabel() const;
void setLabel(const QString label);
void setLabel(QString label);

QColor getColor() const;
void setColor(QColor color);

signals:
void updated();

private:
explicit Cue(TrackId trackId);
Cue(int id, TrackId trackId, CueType type, int position, int length,
int hotCue, QString label);
int hotCue, QString label, QColor color);
void setDirty(bool dirty);
void setId(int id);
void setTrackId(TrackId trackId);
Expand All @@ -67,6 +71,7 @@ class Cue : public QObject {
int m_iLength;
int m_iHotCue;
QString m_label;
QColor m_color;

friend class Track;
friend class CueDAO;
Expand Down
10 changes: 7 additions & 3 deletions src/library/dao/cuedao.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ CuePointer CueDAO::cueFromRow(const QSqlQuery& query) const {
int length = record.value(record.indexOf("length")).toInt();
int hotcue = record.value(record.indexOf("hotcue")).toInt();
QString label = record.value(record.indexOf("label")).toString();
QColor color = QColor::fromRgba(record.value(record.indexOf("color")).toInt());
CuePointer pCue(new Cue(id, trackId, (Cue::CueType)type,
position, length, hotcue, label));
position, length, hotcue, label, color));
m_cues[id] = pCue;
return pCue;
}
Expand Down Expand Up @@ -148,13 +149,14 @@ bool CueDAO::saveCue(Cue* cue) {
if (cue->getId() == -1) {
// New cue
QSqlQuery query(m_database);
query.prepare("INSERT INTO " CUE_TABLE " (track_id, type, position, length, hotcue, label) VALUES (:track_id, :type, :position, :length, :hotcue, :label)");
query.prepare("INSERT INTO " CUE_TABLE " (track_id, type, position, length, hotcue, label, color) VALUES (:track_id, :type, :position, :length, :hotcue, :label, :color)");
query.bindValue(":track_id", cue->getTrackId().toVariant());
query.bindValue(":type", cue->getType());
query.bindValue(":position", cue->getPosition());
query.bindValue(":length", cue->getLength());
query.bindValue(":hotcue", cue->getHotCue());
query.bindValue(":label", cue->getLabel());
query.bindValue(":color", cue->getColor().rgba());

if (query.exec()) {
int id = query.lastInsertId().toInt();
Expand All @@ -172,7 +174,8 @@ bool CueDAO::saveCue(Cue* cue) {
"position = :position,"
"length = :length,"
"hotcue = :hotcue,"
"label = :label"
"label = :label,"
"color = :color"
" WHERE id = :id");
query.bindValue(":id", cue->getId());
query.bindValue(":track_id", cue->getTrackId().toVariant());
Expand All @@ -181,6 +184,7 @@ bool CueDAO::saveCue(Cue* cue) {
query.bindValue(":length", cue->getLength());
query.bindValue(":hotcue", cue->getHotCue());
query.bindValue(":label", cue->getLabel());
query.bindValue(":color", cue->getColor().rgba());

if (query.exec()) {
cue->setDirty(false);
Expand Down
2 changes: 1 addition & 1 deletion src/library/trackcollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "util/assert.h"

// static
const int TrackCollection::kRequiredSchemaVersion = 26;
const int TrackCollection::kRequiredSchemaVersion = 27;

TrackCollection::TrackCollection(UserSettingsPointer pConfig)
: m_pConfig(pConfig),
Expand Down
53 changes: 8 additions & 45 deletions src/waveform/renderers/waveformmark.cpp
Original file line number Diff line number Diff line change
@@ -1,62 +1,25 @@
#include <QtDebug>

#include "waveformmark.h"
#include "skin/skincontext.h"
#include "waveform/renderers/waveformmarkproperties.h"

#include "waveformwidgetrenderer.h"
#include "control/controlobject.h"
#include "control/controlproxy.h"
#include "widget/wskincolor.h"
#include "waveformmark.h"

WaveformMark::WaveformMark()
: m_pPointCos(nullptr) {
: m_iHotCue(-1) {
}

WaveformMark::~WaveformMark() {
delete m_pPointCos;
WaveformMark::WaveformMark(int hotCue)
: m_iHotCue(hotCue) {
}

void WaveformMark::setup(const QString& group, const QDomNode& node,
const SkinContext& context,
const WaveformSignalColors& signalColors) {
QString item = context.selectString(node, "Control");
if (!item.isEmpty()) {
m_pPointCos = new ControlProxy(group, item);
}

m_color = context.selectString(node, "Color");
if (!m_color.isValid()) {
// As a fallback, grab the color from the parent's AxesColor
m_color = signalColors.getAxesColor();
qDebug() << "Didn't get mark <Color>, using parent's <AxesColor>:" << m_color;
} else {
m_color = WSkinColor::getCorrectColor(m_color);
}

m_textColor = context.selectString(node, "TextColor");
if (!m_textColor.isValid()) {
// Read the text color, otherwise use the parent's BgColor.
m_textColor = signalColors.getBgColor();
qDebug() << "Didn't get mark <TextColor>, using parent's <BgColor>:" << m_textColor;
m_pPointCos = std::make_unique<ControlProxy>(group, item);
}

QString markAlign = context.selectString(node, "Align");
if (markAlign.contains("bottom", Qt::CaseInsensitive)) {
m_align = Qt::AlignBottom;
} else {
m_align = Qt::AlignTop; // Default
}

m_text = context.selectString(node, "Text");
m_pixmapPath = context.selectString(node, "Pixmap");
if (!m_pixmapPath.isEmpty()) {
m_pixmapPath = context.getSkinPath(m_pixmapPath);
}
}

// called from WaveformMarkSet::setup() for hot cues
// TODO(XXX): subclass and override WaveformMark::setup
void WaveformMark::setKeyAndIndex(const ConfigKey& key, int i) {
DEBUG_ASSERT(m_pPointCos == NULL);
m_pPointCos = new ControlProxy(key);
m_text = m_text.arg(i);
m_properties = WaveformMarkProperties(node, context, signalColors);
}
37 changes: 20 additions & 17 deletions src/waveform/renderers/waveformmark.h
Original file line number Diff line number Diff line change
@@ -1,39 +1,42 @@
#ifndef WAVEFORMMARK_H
#define WAVEFORMMARK_H

#include <QString>
#include <QDomNode>
#include <QImage>
#include <QColor>

#include "preferences/usersettings.h"
#include "skin/skincontext.h"
#include "control/controlproxy.h"
#include "util/memory.h"

class ControlProxy;
class QDomNode;
#include "waveform/renderers/waveformmarkproperties.h"

class SkinContext;
class WaveformSignalColors;

class WaveformMark {
public:
WaveformMark();
Copy link
Contributor

Choose a reason for hiding this comment

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

The default constructor has become obsolete now, because C++ generates one for you. If you want to keep it for documentation purposes then use
WaveformMark() = default;
and delete the implementation. I like C++1x ;)

~WaveformMark();
WaveformMark(int hotCue);

void setup(const QString& group, const QDomNode& node,
const SkinContext& context,
const WaveformSignalColors& signalColors);
void setKeyAndIndex(const ConfigKey& key, int i);

private:
ControlProxy* m_pPointCos;
std::unique_ptr<ControlProxy> m_pPointCos;

const WaveformMarkProperties& getProperties() const { return m_properties; };
void setProperties(const WaveformMarkProperties& properties) { m_properties = properties; };

QColor m_color;
QColor m_textColor;
QString m_text;
Qt::Alignment m_align;
QString m_pixmapPath;
int getHotCue() const { return m_iHotCue; };
void setHotCue(int hotCue) { m_iHotCue = hotCue; };

private:
WaveformMarkProperties m_properties;
int m_iHotCue;
QImage m_image;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the image is another intrinsic and copyable property that belongs to WaveformMarkProperties. With this modification generateMarkImage() can operate on WaveformMarkProperties directly.

But I have not checked all occurrences, you may know better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this worth it? Making m_image part of WaveformMarkProperties means that in WaveformRenderMark::slotCuesUpdated we are copying it just to regenerate it afterwards.

Also, it is strange to me to think about it as a "property" in the same sense than color or text. Conceptually, for me it should be a member of WaveforMark.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's ok for me. As I mentioned, I didn't take into account all the details.

On 21.08.2016 15:38, Ferran Pujol Camins wrote:

In src/waveform/renderers/waveformmark.h
#992 (comment):

  • QColor m_color;
  • QColor m_textColor;
  • QString m_text;
  • Qt::Alignment m_align;
  • QString m_pixmapPath;
  • const WaveformMarkProperties& properties() const;
  • void setProperties(const WaveformMarkProperties& properties);
  • private:
  • WaveformMarkProperties m_properties;
    QImage m_image;

Is this worth it? Making m_image part of WaveformMarkProperties means that
in WaveformRenderMark::slotCuesUpdated we are copying it just to
regenerate it afterwards.

Also, it is strange to me to think about it as a "property" in the same
sense than color or text. Conceptually, for me it should be a member of
WaveforMark.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mixxxdj/mixxx/pull/992/files/2f8863b7b8266565a97581fd38cfa6e87ab0ac36#r75595330,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABZ-alocrPGQJL7xYxSVnZdXHpOTqALwks5qiFTvgaJpZM4Jj7vm.


friend class WaveformMarkSet;
friend class WaveformRenderMark;
friend class WOverview;
};

typedef QSharedPointer<WaveformMark> WaveformMarkPointer;

#endif // WAVEFORMMARK_H
38 changes: 38 additions & 0 deletions src/waveform/renderers/waveformmarkproperties.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#include "skin/skincontext.h"
#include "waveform/renderers/waveformsignalcolors.h"
#include "widget/wskincolor.h"

#include "waveform/renderers/waveformmarkproperties.h"

WaveformMarkProperties::WaveformMarkProperties(const QDomNode& node,
const SkinContext& context,
const WaveformSignalColors& signalColors) {
m_color = context.selectString(node, "Color");
if (!m_color.isValid()) {
// As a fallback, grab the color from the parent's AxesColor
m_color = signalColors.getAxesColor();
qDebug() << "Didn't get mark <Color>, using parent's <AxesColor>:" << m_color;
} else {
m_color = WSkinColor::getCorrectColor(m_color);
}

m_textColor = context.selectString(node, "TextColor");
if (!m_textColor.isValid()) {
// Read the text color, otherwise use the parent's BgColor.
m_textColor = signalColors.getBgColor();
qDebug() << "Didn't get mark <TextColor>, using parent's <BgColor>:" << m_textColor;
}

QString markAlign = context.selectString(node, "Align");
if (markAlign.contains("bottom", Qt::CaseInsensitive)) {
m_align = Qt::AlignBottom;
} else {
m_align = Qt::AlignTop; // Default
}

m_text = context.selectString(node, "Text");
m_pixmapPath = context.selectString(node, "Pixmap");
if (!m_pixmapPath.isEmpty()) {
m_pixmapPath = context.getSkinPath(m_pixmapPath);
}
}
24 changes: 24 additions & 0 deletions src/waveform/renderers/waveformmarkproperties.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#ifndef WAVEFORMMARKPROPERTIES_H
#define WAVEFORMMARKPROPERTIES_H

#include <QColor>
#include <QDomNode>

class SkinContext;
class WaveformSignalColors;

class WaveformMarkProperties final {
public:
WaveformMarkProperties() = default;
WaveformMarkProperties(const QDomNode& node,
const SkinContext& context,
const WaveformSignalColors& signalColors);

QColor m_color;
QColor m_textColor;
QString m_text;
Qt::Alignment m_align;
QString m_pixmapPath;
};

#endif // WAVEFORMMARKPROPERTIES_H
Loading