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.

Sign up for GitHub

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

Color enhance #104

Merged
merged 12 commits into from
Oct 20, 2018
Merged

Color enhance #104

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4de8a95
ColorBox replaced by ColorButton.
gbm19 Oct 8, 2018
291e77c
Define a macro to save color values including alpha
gbm19 Oct 8, 2018
da5e955
Set basic colors in QColorDialog from internal color list
gbm19 Oct 8, 2018
a3a19f4
Remove redundant numPredefinedColors
gbm19 Oct 10, 2018
d745453
Avoid defining additional color variables
gbm19 Oct 10, 2018
1cd7434
Replace macro by inline function
gbm19 Oct 10, 2018
ea7252c
Additional basic colors
gbm19 Oct 10, 2018
7a2ea46
Avoid possible crash
gbm19 Oct 11, 2018
d3bee93
Remove references to ColorBox
gbm19 Oct 11, 2018
bd0f884
Remove use of colorIndex where possible
gbm19 Oct 12, 2018
1363d50
Save colors as hex values in .sciproj files
gbm19 Oct 12, 2018
52acd14
Save color settings as hex values
gbm19 Oct 12, 2018
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
4 changes: 2 additions & 2 deletions libscidavis/src/ApplicationWindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
#include "Convolution.h"
#include "Correlation.h"
#include "CurveRangeDialog.h"
#include "ColorBox.h"
#include "ColorButton.h"
#include "QwtHistogram.h"
#include "OpenProjectDialog.h"
#include "IconLoader.h"
Expand Down Expand Up @@ -9676,7 +9676,7 @@ Graph* ApplicationWindow::openGraph(ApplicationWindow* app, MultiLayer *plot,
if (d_file_version <= 77)
{
int temp_index = convertOldToNewColorIndex(curve[15].toInt());
ag->updateVectorsLayout(curveID, ColorBox::color(temp_index), curve[16].toInt(), curve[17].toInt(),
ag->updateVectorsLayout(curveID, ColorButton::color(temp_index), curve[16].toInt(), curve[17].toInt(),
curve[18].toInt(), curve[19].toInt(), 0, curve[20], curve[21]);
}
else
Expand Down
13 changes: 6 additions & 7 deletions libscidavis/src/AxesDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "ApplicationWindow.h"
#include "AxesDialog.h"
#include "TextDialog.h"
#include "ColorBox.h"
#include "Graph.h"
#include "Grid.h"
#include "Plot.h"
Expand Down Expand Up @@ -233,10 +232,10 @@ void AxesDialog::initGridPage()

rightLayout->addWidget( new QLabel(tr( "Line Color" )), 1, 0 );

boxColorMajor = new ColorBox(0);
boxColorMajor = new ColorButton();
rightLayout->addWidget( boxColorMajor, 1, 1);

boxColorMinor = new ColorBox(0);
boxColorMinor = new ColorButton();
boxColorMinor->setDisabled(true);
rightLayout->addWidget( boxColorMinor, 1, 2);

Expand Down Expand Up @@ -1050,17 +1049,17 @@ void AxesDialog::updateGrid()
grid->enableX(boxMajorGrid->isChecked());
grid->enableXMin(boxMinorGrid->isChecked());

grid->setMajPenX(QPen(ColorBox::color(boxColorMajor->currentIndex()), boxWidthMajor->value(),
grid->setMajPenX(QPen(boxColorMajor->color(), boxWidthMajor->value(),
Graph::getPenStyle(boxTypeMajor->currentIndex())));
grid->setMinPenX(QPen(ColorBox::color(boxColorMinor->currentIndex()), boxWidthMinor->value(),
grid->setMinPenX(QPen(boxColorMinor->color(), boxWidthMinor->value(),
Graph::getPenStyle(boxTypeMinor->currentIndex())));
} else {
grid->enableY(boxMajorGrid->isChecked());
grid->enableYMin(boxMinorGrid->isChecked());

grid->setMajPenY(QPen(ColorBox::color(boxColorMajor->currentIndex()), boxWidthMajor->value(),
grid->setMajPenY(QPen(boxColorMajor->color(), boxWidthMajor->value(),
Graph::getPenStyle(boxTypeMajor->currentIndex())));
grid->setMinPenY(QPen(ColorBox::color(boxColorMinor->currentIndex()), boxWidthMinor->value(),
grid->setMinPenY(QPen(boxColorMinor->color(), boxWidthMinor->value(),
Graph::getPenStyle(boxTypeMinor->currentIndex())));
}

Expand Down
7 changes: 3 additions & 4 deletions libscidavis/src/AxesDialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class QSpinBox;
class QTabWidget;
class QWidget;
class QStringList;
class ColorBox;
class ColorButton;
class Graph;
class TextFormatButtons;
Expand Down Expand Up @@ -103,9 +102,9 @@ class AxesDialog : public QDialog
QCheckBox* boxMajorGrid;
QCheckBox* boxMinorGrid;
QComboBox* boxTypeMajor;
ColorBox* boxColorMinor;
ColorBox* boxColorMajor;
ColorButton *boxCanvasColor;
ColorButton* boxColorMinor;
ColorButton* boxColorMajor;
ColorButton* boxCanvasColor;
QSpinBox* boxWidthMajor;
QComboBox* boxTypeMinor;
QSpinBox* boxWidthMinor;
Expand Down
105 changes: 104 additions & 1 deletion libscidavis/src/ColorButton.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,83 @@
***************************************************************************/
#include "ColorButton.h"

#include <QColorDialog>
#include <QPalette>
#include <QPushButton>
#include <QHBoxLayout>
#include <QFrame>

const QColor ColorButton::colors[] = {
QColor(Qt::black),
QColor(Qt::red),
QColor(Qt::green),
QColor(Qt::blue),
QColor(Qt::cyan),
QColor(Qt::magenta),
QColor(Qt::yellow),
QColor(Qt::darkYellow),
QColor(Qt::darkBlue),
QColor(Qt::darkMagenta),
QColor(Qt::darkRed),
QColor(Qt::darkGreen),
QColor(Qt::darkCyan),
QColor("#0000A0"),
QColor("#FF8000"),
QColor("#8000FF"),
QColor("#FF0080"),
QColor(Qt::white),
QColor(Qt::lightGray),
QColor(Qt::gray),
QColor("#FFFF80"),
QColor("#80FFFF"),
QColor("#FF80FF"),
QColor(Qt::darkGray),
// additional colors from figure 6 in doi:10.1016/j.csda.2008.11.033
QColor("#023fa5"),
QColor("#4a6fe3"),
QColor("#11c638"),
QColor("#0fcfc0"),
QColor("#8e063b"),
QColor("#d33f6a"),
QColor("#ef9708"),
QColor("#f79cd4"),
QColor("#7d87b9"),
QColor("#8595e1"),
QColor("#8dd593"),
QColor("#9cded6"),
QColor("#bb7784"),
QColor("#e07b91"),
QColor("#f0b98d"),
QColor("#f6c4e1"),
QColor("#bec1d4"),
QColor("#b5bbe3"),
QColor("#c6dec7"),
QColor("#d5eae7"),
QColor("#d6bcc0"),
QColor("#e6afb9"),
QColor("#ead3c6"),
QColor("#f3e1eb"),
};

const unsigned int ColorButton::colors_count=sizeof(colors)/sizeof(colors[0]);

ColorButton::ColorButton(QWidget *parent) : QWidget(parent)
{
init();
}

void ColorButton::init()
{
// QColorDialog has 48 basic colors in a 6x8 grid. Using:
// for (int i=0; i < colors_count; i++) {QColorDialog::setStandardColor(i, colors[i].rgb());};
// arranges the colors in columns of 6. Instead arrange them in rows of 8 colors each.
Copy link
Owner

Choose a reason for hiding this comment

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

Your comment is still slightly confusing - given a 6x8 grid, columns of 6 is the same thing as rows of 8.
You could simply say "transpose the 6x8 grid", which is what you're actually doing.

for (int i=0; i<8; i++)
{
for (int j=0; j<6; j++)
{
QColorDialog::setStandardColor(j+6*i, colors[i+8*j].rgb());
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

The magic numbers 8 and 3 bother me a bit. I assume that maybe the dialog is perhaps 8x3 or something? At very least, there should be a static_assert that 8*3 <= colors_count.

Also what happens to standard colours 3,4,5, 9,10,11, ...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

QColorDialog has 48 basic colors in a 6x8 grid.
Using:
for (int i=0; i < colors_count; i++) {QColorDialog::setStandardColor(i, colors[i].rgb());};
arranges the colors in columns of 6. I prefer to arrange them in rows of 8 colors each

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe a comment along the lines of rearranging the stand ard layout of standard colours might be useful. It puzzled me why you were only allocating half the colours.

The static_assert might also be useful, in case someone decides to remove some from the list, or monkey around with the magic numbers.

Copy link
Owner

@highperformancecoder highperformancecoder Oct 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm still keen on the static_assert:

static_assert(8*6 <= colours_count, "not enough default colors");

Better still, name the 8 and the 6, and use the names both in the loop and the assert. That way, as soon as anyone touches this in an invalid way, it will thow up a compiler error.

const int btn_size = 28;
selectButton = new QPushButton(QPixmap(":/palette.xpm"), QString(), this);
selectButton->setMinimumWidth(btn_size);
Expand All @@ -61,22 +126,60 @@ void ColorButton::init()
setMaximumWidth(3*btn_size);
setMaximumHeight(btn_size);

connect(selectButton, SIGNAL(clicked()), this, SIGNAL(clicked()));
connect(selectButton, SIGNAL(clicked()), this, SLOT(pickColor()));
}

void ColorButton::setColor(const QColor& c)
{
QPalette pal;
pal.setColor(QPalette::Window, c);
display->setPalette(pal);
emit changed(c);
}

QColor ColorButton::color() const
{
return display->palette().color(QPalette::Window);
}

unsigned int ColorButton::colorIndex(const QColor& c)
{
const QColor *ite = std::find(std::begin(colors), std::end(colors), c);
if (ite->isValid())
return (ite - colors);
else
return c.rgba();
}
highperformancecoder marked this conversation as resolved.
Show resolved Hide resolved

QColor ColorButton::color(unsigned int colorIndex)
{
if (colorIndex < colors_count)
return colors[colorIndex];
else
{
QColor qc = QColor::fromRgba(colorIndex);
if (qc.isValid())
return qc;
else
return QColor(Qt::black); // default color is black.
}
}

bool ColorButton::isValidColor(const QColor& c)
{
const QColor *ite = std::find(std::begin(colors), std::end(colors), c);
return (ite->isValid());
Copy link
Owner

@highperformancecoder highperformancecoder Oct 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Dereferencing the end of a sequence (as what happens if the colour is not found) has undefined behaviour, and could even cause a null dereference crash.

The return test could be as simple as ite-color < colors_count, or to be more std libraryish ite!=std::end(colors)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Qt takes care of all that trouble.

Copy link
Owner

@highperformancecoder highperformancecoder Oct 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think so. std::find returns an iterator pointing to the end of the sequence, which in this case will be a pointer to one past the end the colour array. ite->isValid() is completely undefined - the compiler could initialise the memory past the end of the array to zero, or some garbage pattern, or it may put a properly initialised default QColor, or it may remain uninitialised to some random value.

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 guess Qt appends a properly initialised QColor(Invalid) at end of array at the constructor, because that is what I get when I test the code.

Copy link
Owner

Choose a reason for hiding this comment

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

It's not Qt. It's your compiler. Someone else's compiler may do it differently. Next year, your compiler may do it differently. It is not good to rely on it.

}

QSize ColorButton::sizeHint () const
{
return QSize(4*btn_size, btn_size);
}

void ColorButton::pickColor()
{
QColor c = QColorDialog::getColor(color(), this, "Select color", QColorDialog::DontUseNativeDialog | QColorDialog::ShowAlphaChannel);
if ( !c.isValid() || c == color() )
return;
setColor ( c ) ;
}
20 changes: 20 additions & 0 deletions libscidavis/src/ColorButton.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ class QPushButton;
class QHBoxLayout;
class QFrame;

#if QT_VERSION >= 0x050000
static inline QString COLORNAME( QColor c ) { return c.name(QColor::HexArgb); }
#else
static inline QString COLORNAME( QColor c ) { return ('#'+QString::number(c.rgba(),16)); }
#endif

highperformancecoder marked this conversation as resolved.
Show resolved Hide resolved
//! A button used for color selection
/**
* This button contains two widgets:
Expand All @@ -52,6 +58,14 @@ class ColorButton : public QWidget
void setColor(const QColor& c);
//! Get the color of the display part
QColor color() const;
//! Return the index for a given color
static unsigned int colorIndex(const QColor& c);
//! Return the color at index 'colorindex'
static QColor color(unsigned int colorIndex);
//! Returns TRUE if the color is included in the color box, otherwise returns FALSE.
static bool isValidColor(const QColor& color);
//! The number of predefined colors
static const unsigned int colors_count;
QSize sizeHint() const;

private:
Expand All @@ -61,10 +75,16 @@ class ColorButton : public QWidget
signals:
//! Signal clicked: This is emitted when the selection button is clicked
void clicked();
void changed(QColor);

protected:
//! Initialize the widget (called from constructor)
void init();
//! Array containing the 24 predefined colors
static const QColor colors[];

private slots:
void pickColor();

private:
int btn_size;
Expand Down
7 changes: 3 additions & 4 deletions libscidavis/src/ConfigDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "Graph.h"
#include "Matrix.h"
#include "ColorButton.h"
#include "ColorBox.h"

#include <QLocale>
#include <QPushButton>
Expand Down Expand Up @@ -697,8 +696,8 @@ void ConfigDialog::initFittingPage()

lblPeaksColor = new QLabel();
multiPeakLayout->addWidget(lblPeaksColor);
boxPeaksColor = new ColorBox(0);
boxPeaksColor->setCurrentIndex(app->peakCurvesColor);
boxPeaksColor = new ColorButton();
boxPeaksColor->setColor(app->peakCurvesColor);
multiPeakLayout->addWidget(boxPeaksColor);

groupBoxFitParameters = new QGroupBox();
Expand Down Expand Up @@ -1184,7 +1183,7 @@ void ConfigDialog::apply()
app->fitPoints = generatePointsBox->value();
app->generateUniformFitPoints = generatePointsBtn->isChecked();
app->generatePeakCurves = groupBoxMultiPeak->isChecked();
app->peakCurvesColor = boxPeaksColor->currentIndex();
app->peakCurvesColor = boxPeaksColor->colorIndex(boxPeaksColor->color());
app->fit_scale_errors = scaleErrorsBox->isChecked();
app->d_2_linear_fit_points = linearFit2PointsBox->isChecked();
app->saveSettings();
Expand Down
3 changes: 1 addition & 2 deletions libscidavis/src/ConfigDialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class QLabel;
class QRadioButton;
class QListWidget;
class ColorButton;
class ColorBox;

#ifndef TS_PATH
#define TS_PATH (qApp->applicationDirPath() + "/translations")
Expand Down Expand Up @@ -171,7 +170,7 @@ private slots:
QGroupBox *groupBoxFittingCurve, *groupBoxFitParameters;
QRadioButton *samePointsBtn, *generatePointsBtn;
QGroupBox *groupBoxMultiPeak;
ColorBox *boxPeaksColor;
ColorButton *boxPeaksColor;
QLabel *lblScriptingLanguage;
QComboBox *boxScriptingLanguage;
QCheckBox *boxAntialiasing, *boxAutoscale3DPlots, *boxTableComments;
Expand Down
4 changes: 2 additions & 2 deletions libscidavis/src/Convolution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#include "MultiLayer.h"
#include "Plot.h"
#include "PlotCurve.h"
#include "ColorBox.h"
#include "ColorButton.h"
#include "core/column/Column.h"

#include <QMessageBox>
Expand Down Expand Up @@ -162,7 +162,7 @@ void Convolution::addResultCurve()

DataCurve *c = new DataCurve(d_table, d_table->colName(cols), d_table->colName(cols2));
c->setData(x_temp, d_x, d_n);
c->setPen(QPen(ColorBox::color(d_curveColorIndex), 1));
c->setPen(QPen(ColorButton::color(d_curveColorIndex), 1));
ml->activeGraph()->insertPlotItem(c, Graph::Line);
ml->activeGraph()->updatePlot();
}
Expand Down
4 changes: 2 additions & 2 deletions libscidavis/src/Correlation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#include "MultiLayer.h"
#include "Plot.h"
#include "PlotCurve.h"
#include "ColorBox.h"
#include "ColorButton.h"
#include <QMessageBox>
#include <QLocale>
#include "core/column/Column.h"
Expand Down Expand Up @@ -175,7 +175,7 @@ void Correlation::addResultCurve()

DataCurve *c = new DataCurve(d_table, d_table->colName(cols), d_table->colName(cols2));
c->setData(&x_temp[0], &y_temp[0], rows);
c->setPen(QPen(ColorBox::color(d_curveColorIndex), 1));
c->setPen(QPen(ColorButton::color(d_curveColorIndex), 1));
ml->activeGraph()->insertPlotItem(c, Graph::Line);
ml->activeGraph()->updatePlot();
}
10 changes: 5 additions & 5 deletions libscidavis/src/ExpDecayDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
***************************************************************************/
#include "ExpDecayDialog.h"
#include "Graph.h"
#include "ColorBox.h"
#include "ColorButton.h"
#include "ApplicationWindow.h"
#include "Fit.h"
#include "ExponentialFit.h"
Expand Down Expand Up @@ -111,9 +111,9 @@
gl1->addWidget(boxStart, 5, 1);

gl1->addWidget(new QLabel(tr("Color")), 6, 0 );
boxColor = new ColorBox();
boxColor->setColor(QColor(Qt::red));
gl1->addWidget(boxColor, 6, 1);
btnColor = new ColorButton();
btnColor->setColor(QColor(Qt::red));
gl1->addWidget(btnColor, 6, 1);

gb1->setLayout(gl1);

Expand Down Expand Up @@ -232,7 +232,7 @@ void ExpDecayDialog::fit()

if (fitter->setDataFromCurve(boxName->currentText(), boxStart->text().toDouble(), c->maxXValue()))
{
fitter->setColor(boxColor->currentIndex());
fitter->setColor(ColorButton::colorIndex(btnColor->color()));
fitter->scaleErrors(app->fit_scale_errors);
fitter->setOutputPrecision(app->fit_output_precision);
fitter->generateFunction(app->generateUniformFitPoints, app->fitPoints);
Expand Down
Loading