Problem with a color class

[zorro68]

I am using Visual Studio 2005 and Qt 4.3.1.

When I call my class "colors" like this:

colors col;
int r=col.getRed(3);

the value of r is bad (always 0). ¿¿Someone knows why??


My colors.h



class colors
{
public:
colors();
~colors();

int getRed(int num);
enum{max_colors=17};

private:
struct ColorData
{
int num;
QString name;
int R;
int G;
int B;
}Propiedades[max_colors],*p;
};


and my colors.cpp



#include "colors.h"

colors::colors()
{
ColorData Propiedades[max_colors] =
{
{ 0, "black", 0, 0, 0},
{ 1, "white",255,255,255},
{ 2, "darkGray",128,128,128},
{ 3, "gray",160,160,164},
...
};
p=&Propiedades[0];
}

int colors::getRed(int num)
{
ColorData *q;
q=p;
for (int i=0;i<=max_colors;i++){
if (num==q->num) return q->R;
q++;
}
return 0;
}

[jpn]

May I ask why not use QColor?

[zorro68]

As you know (other post), I'm trying to programm a combobox with colors. But I need only some colors and when I programm QColor("name of color") :


QStringList colorNames;
colorNames <<"black"<<"white"<<"darkGray"....
...
pixmap.fill(QColor(name));


some color are bad (for example darkyellow is black for me). So I programm colors with a structure:



struct colortype {QString name;int r; int g; int b;};
colortype colors[max_color+1];
colors[0].name="black";colors[0].r=0;colors[0].g=0;colors[0].b=0;
colors[1].name="white";colors[1].r=255;colors[1].g=255;colors[1].b=255;
...
pixmap.fill(QColor(colors[cont].r,colors[cont].g,colors[cont].b));


but i need to use this in several classes. So I think to programm my own class for colors (with only the colors i need and in rgb mode).

[zorro68]

Can anybody help me???

[wysota]

Hmm... why don't you use RGB values instead of names then? Take a look at my color combobox (btw. there is no point in reinventing the wheel, you can use QwwColorComboBox in your project provided you won't violate the licence).

[zorro68]

I'm newbie in Qt and I don't know about your project (im looking it and its a good project). But I have two reason for programming my own class:

First, I use the same structure for defining chemical elements, and other properties, that's why a need this structure go fine.
Second, im learning with qt and i know that the most you programm, most you learn.

As you can see at first of this topic, I'm using RGB color but when I call the class don't work correctly (structure variables take bad values).:confused::confused:

Thanks

[wysota]

The structure is weird. It is much tainted with C like code. You should consider something more object oriented. But still you are reinventing the wheel as QColor already does what you want. You could go for a simple QVector<QColor>.

[zorro68]

I don't use QColor class because works bad. When I use:



QStringList colorNames;
colorNames <<"black"<<"white"<<"darkGray".... <-only some colors, not all
...
pixmap.fill(QColor(name)); <- this inside a for loop, this is the cause of the QStringList


the green color is not (0,255,0), is a darkgreen, and darkyellow is black for me.

And if I have the number of one color in another class (0=black, 1=White, ..., not RGB) how can i know its RGB with QColor? (I only knows its ordinal number that is the same as it position in the combo, not its name and not its R, G or B)

[wysota]

So why don't you omit those colors which you don't like or not use names at all? You can create a QColor by passing R, G and B components to QColor constructor.

If you really wish to have a separate structure holding the components (which is completely unnecessary as that's exactly what QColor does), why don't you do it like this?

struct Color{
int red;
int green;
int blue;
QString name;
};
QVector<Color> colors;
foreach(Color col, colors){
QColor c(col.red, col.green, col.blue);
QPixmap px(16,16);
px.fill(c);
comboBox->addItem(QIcon(px), col.name);
}

[zorro68]

I think you don't understand me (excuse me for my bad english), so i'm trying to explain better:

If i used color names like this:


QStringList colorNames;
colorNames <<"black"<<"white"<<"darkGray".... <-only some colors, not all
...
foreach (QString name, colorName){
pixmap.fill(QColor(name));
CMBcolor[i]->addItem(QIcon(pixmap),"", QColor(name));
}

the green color don't display (0,255,0), is a darkgreen, and darkyellow is black for me.
I don't know if this is a bug or not but the combo displays bad colors for these two (green and darkyellow, the other i use are well but probably there will be other colors that fails)



So why don't you omit those colors which you don't like ...


I programm the QStringList to avoid those colors i don't like (I only want 16 colors - white, black, red, darkRed, green, darkGreen, blue, darkBlue, cyan, darkCyan, magenta, darkMagenta, yellow, darkYellow, gray, darkGray, lightGray)



... or not use names at all?


I can avoid names of colors (I use them to complete the structure). But this is not the explanation why don't work my structure.


struct ColorData
{
int num;
QString name;
int R;
int G;
int B;
}Propiedades[max_colors],*p;

...

ColorData Propiedades[max_colors] =
{
{ 0, "black", 0, 0, 0},
{ 1, "white",255,255,255},
{ 2, "darkGray",128,128,128},
{ 3, "gray",160,160,164},
...
};
p=&Propiedades[0];




You can create a QColor by passing R, G and B components to QColor constructor.


Yes, i can. But i have to create the new colors each time i have to use them. So this is the reason to create a class with my colors.



If you really wish to have a separate structure holding the components (which is completely unnecessary as that's exactly what QColor does), why don't you do it like this?


Because if i pick the combo i need to know the r g b components of the color i have picked, and with this code i have to redefined my colors again (in other class). And I don't know how to obtain the rgb components of the color when i click in the combo (the selected item of the combo).

So if my class works I wouldn't be any problem. In the class i have clasified the colors for a single number what is the same that the position that the colors is in the combo. For example, if the combo return 1, i know that is the color white and i know elsewhere of the program the r,g,b components of this color.



connect(CMBcolor,SIGNAL(currentIndexChanged(int)), this,SLOT(CMBcolor_change(int)));


The current index is known in CMBcolor_change by the int variable. And with this integer i obtain the rgb components. With QColor i don't know how to do this because QColor use the name or the r,g,b but not the index.

As you can see i don't use the name of the color, as i say this is to complete the structure.

By all this i have decided to bulid the class.

Excuse me for my english and i hope you understand me.

[Rembobo]

Hello, I think


colors::colors()
{
ColorData Propiedades[max_colors] =// this is local (constructor) variable, not class instance Propiedades.
{
{ 0, "black", 0, 0, 0},
{ 1, "white",255,255,255},
{ 2, "darkGray",128,128,128},
{ 3, "gray",160,160,164},
...
};
p=&Propiedades[0]; // p now points to local (ctor) variable which will be diallocated
//right now.
}

int colors::getRed(int num)
{
ColorData *q;
q=p; // p points to deallocated memory.
for (int i=0;i<=max_colors;i++){// bad, must be "i < max_colors" or even "i < colors::max_colors".
if (num==q->num) return q->R;
q++;
}
return 0;
}

[zorro68]

Yes, I think that I am pointing to a deallocated variable, because p takes extrange values in the debugger. But how to do it well? How to convert this local (constructor) variable to a class instance? (Think that Im newbie in Qt and C++. I programm in Fortran, visual basic, c...)

Thanks

[wysota]

Yes, i can. But i have to create the new colors each time i have to use them.
No, if you store them as QColor objects in the first place.


Because if i pick the combo i need to know the r g b components of the color i have picked, and with this code i have to redefined my colors again (in other class). And I don't know how to obtain the rgb components of the color when i click in the combo (the selected item of the combo).
I really don't see a problem.

QMap<QString, QColor> colors; // or simply QList<QColor> if you don't want the names
colors["white"] = QColor(255,255,255);
colors["red"] = QColor(255,0,0);
//...

for(QMap<QString,QColor>::const_iterator it = colors.begin(); it!=colors.end();++it){
comboBox->addItem(...);
}
and then when you want to identify the color:

int index = comboBox->currentIndex();
QColor col = colors[colors.keys().at(index)];

Or simply use the ability of combobox to store custom data with each item:

QList<QColor> colors;
//...
foreach(QColor c, colors){
comboBox->addItem(...);
int index = comboBox->count()-1;
comboBox->setItemData(index, c);
}
and then use QComboBox::itemData to fetch the color directly from the combobox.


So if my class works I wouldn't be any problem.
Sure. You're just wasting time fixing something that you completely don't need, because the combobox already provides everything you might want. Oh, and about doubling data - you're doubling it with your structure as well. First you hold it in your structure and then QComboBox is creating its own items. You could of course avoid that by using a simple model (like I do with my color combobox class), but in your case I'd simply ignore the redundancy - there is no point in making things too complex.

[zorro68]

Ok wysota, sorry for my ignorance. I'm going to program like you say. I'm proving with the first sugerence and i have a problem:


for(QMap<QString,QColor>::const_iterator it = colors.begin(); it!=colors.end();++it){
pixmap.fill(it->second);
CMBcolor[i]->addItem(QIcon(pixmap),"",it->second);
}

this give me an error for it->second (second not a member of QColor). I have read that it->first is the QString and it->second the QColor, but don't work. Why?

[wysota]

I'm going to program like you say.
I don't say you should do as I said. I only said you were reinventing the wheel and wasting your precious time for things you don't really need. But the final choice is yours.


I'm proving with the first sugerence and i have a problem:


for(QMap<QString,QColor>::const_iterator it = colors.begin(); it!=colors.end();++it){
pixmap.fill(it->second);
CMBcolor[i]->addItem(QIcon(pixmap),"",it->second);
}

this give me an error for it->second (second not a member of QColor). I have read that it->first is the QString and it->second the QColor, but don't work. Why?
You should use it.key() and it.value() instead. Operator * of the iterator (which you are using behind the scenes by calling "->") returns the value of the map item (look at QMap::const_iterator docs) - in this case QColor.

[zorro68]

You have convinced me because i don't like to waste computer memory...

I have prove with it->value()


QMap<QString, QColor> colors;
colors["black"] = QColor(0,0,0);
colors["white"] = QColor(255,255,255);
colors["red"] = QColor(255,0,0);
colors["darkGray"]=QColor(128,128,128);
...

for(QMap<QString,QColor>::const_iterator it = colors.begin(); it!=colors.end();++it){
pixmap.fill(it->value());
CMBcolor[i]->addItem(QIcon(pixmap),"",it->value());
}


But all colors are diferents blue and black. Im using Qt 4.3.1

[wysota]

What is CMBcolor? An array of combobox pointers? How did you create "pixmap"?

The following works well for me:

#include <QComboBox>
#include <QApplication>
#include <QList>
#include <QColor>
#include <QPixmap>

int main(int argc, char **argv){
QApplication app(argc, argv);
QComboBox box;
QList<QColor> colors;
colors << QColor(255, 255, 255) << QColor(255, 0, 0) << QColor(128, 128, 128) << QColor(0,0,0);
QPixmap px(16,16);
foreach(QColor color, colors){
px.fill(color);
box.addItem(px, QString::null, color);
}
box.show();
return app.exec();
}

[zorro68]

What is CMBcolor? An array of combobox pointers?

Yes


How did you create "pixmap"?


int size = CMBcolor[i]->style()->pixelMetric(QStyle::PM_SmallIconSize);
QPixmap pixmap(size, size);

As you know, this is the same as QPixmap pixmap(16,16) <- I have proved with this too

Now i'm going to prove this.

[zorro68]

Yes, this works fine if there is a simple function. But now I'm going to use the "colors" variable in another function and i have an error because this function cannot "see" this variable.
So I put in my .h file the definition of the "colors" variable like this: QList<QColor *> colors; (¿is this ok?)
and I'm going to initialize theirs values in .cpp file with this
colors << QColor(0,0,0) << QColor(255,255,255) << ...
but this not work because the << operator don't work with pointers (I think).
How can i fill the values of colors?

[wysota]

Yes, this works fine if there is a simple function. But now I'm going to use the "colors" variable in another function and i have an error because this function cannot "see" this variable.
So make it global to the parent scope (for example by making it a member variable) or global to the application or pass it as an argument to where you want to use it.


So I put in my .h file the definition of the "colors" variable like this: QList<QColor *> colors; (¿is this ok?)
No. Should be "extern QList<QColor> colors;" (and forget the pointer, there is no point using it and you're wasting additionally 4/8 bytes per item). Then create the variable in one of your .cpp files without the extern keyword.


and I'm going to initialize theirs values in .cpp file with this
colors << QColor(0,0,0) << QColor(255,255,255) << ...
but this not work because the << operator don't work with pointers (I think).
How can i fill the values of colors?

Don't use pointers :)

[zorro68]

Should be "extern QList<QColor> colors;" . Then create the variable in one of your .cpp files without the extern keyword.

So you say that i have to write "extern QList<QColor> colors;" in my .h file out of the class (because .h file conteins a class) and "QList<QColor> colors"; in the function where i want to use this variable in the .cpp file.??

Note that i have defined this variable in the constructor of a class and using in other functions of the class.

[zorro68]

I have a lots of troubles with the extern variable and to avoid this extern variable, i have try this:


foreach(QColor color, colors){
CMBcolor[i]->addItem(QString::null,color);
int index = CMBcolor[i]->count()-1;
pixmap.fill(color);
CMBcolor[i]->setItemData(index, pixmap, Qt::DecorationRole);
}

and fill all my combos correctly (I think this is more elegant). But when I want to know the clicked color i program (in other functions):


QColor col = CMBcolor[sup]->itemData(ncolor,Qt::UserRole);
(ncolor is the currentitem in the combo, sup is the combo i have picked)

But I obtain an error. QColor is not QVariant. I have proved this:


QVariant col = CMBcolor[sup]->itemData(ncolor,Qt::UserRole);

and compile ok but how can i obtain the r chanel, g chanel and b chanel of the color from col.

Note that i would also have this problem with the other way too.

[wysota]

So you say that i have to write "extern QList<QColor> colors;" in my .h file out of the class (because .h file conteins a class) and "QList<QColor> colors"; in the function where i want to use this variable in the .cpp file.??
No, not in a function. Outside all functions - this will make it global to the application. And the extern statement just says "hey you compiler! There is a 'QList<QColor> colors' variable somewhere out there so you can allow functions to use it, it will be available during link time".


Note that i have defined this variable in the constructor of a class and using in other functions of the class.
You should define it in main.cpp (outside main()) and fill it in main().


I have a lots of troubles with the extern variable and to avoid this extern variable, i have try this:


foreach(QColor color, colors){
CMBcolor[i]->addItem(QString::null,color);
int index = CMBcolor[i]->count()-1;
pixmap.fill(color);
CMBcolor[i]->setItemData(index, pixmap, Qt::DecorationRole);
}

and fill all my combos correctly (I think this is more elegant).
Not really. It would be better to subclass QComboBox and do that inside its constructor. And of course then use the subclass instead of QComboBox.



But I obtain an error. QColor is not QVariant. I have proved this:


QVariant col = CMBcolor[sup]->itemData(ncolor,Qt::UserRole);
Of course it's not. You have to convert it to QColor. I suggest you read QVariant docs, especially the section about qvariant_cast.

Maybe you should just use QwwColorComboBox (http://www.wysota.eu.org/wwwidgets/doc/html/qwwcolorcombobox.html)? :)

[zorro68]

At last, the combo works fine.

Wysota thank you very much for your help and for your patience. Think that i am newbie in qt and c++ so i don't know a lot of "easy" ways to do something in qt.

I'm programming an scientific aplication in fortran (this works fine) and I need a grafical interface to introduce data, but to work with windows and linux, so this is why i use qt. I have program in fortran, c, visual basic, ... and i thought that c++ would be like c, but it isn't. Now i have to plot results (data surface) in 3d, so i'm programming an opengl module (marching cubes algorithm) to do this. ¡¡¡¡¡¡¡ C++, qt, opengl, math algorithms all in one, buffff !!!!!!!! and english. In two weeks, i'll go to uk (Cardiff) with a grant to spend 10 month (I hope to learn a good english).

I'll be here soon with other question...

Now a personal question for you and jpn (you can answer if you like), have you programm the qt or do you work for qt? (because you are connected all time as i can see).

Thanks for all

[wysota]

We don't work for Trolltech if that is what you ask.