Function not receiving QPixmap

[nmuntz]

Hi,

I subclassed QLabel so that it supports the clicked() event and I'm trying to create a small image viewer.
The problem is that my setImage function is not receiving the image properly from the connect statement. Instead of actually setting the image, the QLabel sets itself blank (ie it doesnt show anything). I would really appreciate any help on what am I doing wrong.




// connecting signal
connect(labels[imagenum], SIGNAL(clicked(QPixmap&)), this, SLOT(setImage(QPixmap&)));

void MainWindow::setImage(QPixmap &pic)
{
// mainimage is a QLabel
mainimage->setPixmap(pic);
mainimage->repaint();
}



// imagelabel.cpp

#include <QtGui>
#include "imagelabel.h"

ImageLabel::ImageLabel(QWidget *parent) : QLabel(parent)
{
connect(this, SIGNAL(clicked(QPixmap&)), this, SLOT(slotClicked()));
}

void ImageLabel::slotClicked()
{
qDebug() << "clicked";
}

void ImageLabel::mousePressEvent(QMouseEvent *event)
{
emit clicked(local_pixmap);
}



// imagelabel.h


#ifndef IMAGELABEL_H
#define IMAGELABEL_H

#include <QLabel>

class ImageLabel : public QLabel
{
Q_OBJECT
public:
ImageLabel (QWidget *parent = 0);
~ImageLabel() {}

signals:
void clicked(QPixmap&);

public slots:
void slotClicked();

protected:
void mousePressEvent(QMouseEvent *event);

private:
QPixmap local_pixmap;
};

#endif // IMAGELABEL_H


Thanks a lot in advance for your help!

[wysota]

"QPixmap &" and "const QPixmap &" are two very different things. First of all I'd transmit the latter instead of the former. Second of all I would emit clicked() on mouseRelease and not mousePress. Third of all I would enable console support (CONFIG+=console) and see if Qt spits out any warnings. Fourth of all I would use QListView and QStandardItemModel instead of a bunch of labels.

[nmuntz]

Hi Wysota,

Thank you for your answer.
I made the corrections that you suggested but I still have no luck :(

Regarding your suggestion to use a QListView, I am actually using a QScrollArea and a QHBoxLayout to contain the labels.

I enabled CONFIG+=console and at the beggining it was spitting out some errors but I think I corrected all of them since now it is not spitting any however i still have the same problem.

Did I do right the corrections that you suggested?



connect(labels[imagenum], SIGNAL(clicked(const QPixmap&)), this, SLOT(setImage(const QPixmap &)));

void MainWindow::setImage(const QPixmap &pic)
{
mainimage->setPixmap(pic);
mainimage->repaint();
}




// imagelabel.cpp
#include <QtGui>
#include "imagelabel.h"

ImageLabel::ImageLabel(QWidget *parent) : QLabel(parent)
{
connect(this, SIGNAL(clicked(const QPixmap&)), this, SLOT(slotClicked()));
}

void ImageLabel::slotClicked()
{
qDebug() << "clicked";
}

void ImageLabel::mouseReleaseEvent(QMouseEvent *event)
{
emit clicked(local_pixmap);
}



// imagelabel.h
#ifndef IMAGELABEL_H
#define IMAGELABEL_H

#include <QLabel>

class ImageLabel : public QLabel
{
Q_OBJECT
public:
ImageLabel (QWidget *parent = 0);
~ImageLabel() {}

signals:
void clicked(const QPixmap&);

public slots:
void slotClicked();

protected:
void mouseReleaseEvent(QMouseEvent *event);

private:
QPixmap local_pixmap;
};

#endif // IMAGELABEL_H


Thanks in advance.

[nmuntz]

I think my error is somehow in my connect statement, because I just added:



if (pic.isNull())
qDebug() << "it's null";

to my setImage function and its returning null...
so i guess i'm doing something wrong in my connect statement.

is that the right way of passing the QPixmap attribute of a QLabel?
connect(label, SIGNAL(clicked(const QPixmap&)), this, SLOT(setImage(const QPixmap &)))

Thanks in advance.

[wysota]

I don't think you assign "local_pixmap" anywhere. Anyway I would really suggest to use QListView. You may visit http://doc.trolltech.com/qq/ and grab example sources from my article from issue #27 regarding responsive GUIs. The article is about something completely different but there is a good piece of code you can see. Run the watcher example and see what you can obtain easily using a listview. Reacting to clicks is just a matter of connecting to the clicked() or activated() signal of the view.

Here is a small teaser:
2898

[nmuntz]

What do you mean by not assigning local_pixmap anywhere? Isn't the emit emit clicked(local_pixmap) enough?

And I printed out the Qt Quarterly article that you wrote, its a very interesting read, my app is actually suffering a little bit from that so I'll definitely implement one of your solutions !!! The watcher example is really nice and I will reconsider recoding that part using a ListView like you say.

I'm not sure what else to do with this. I have tried everything with no luck.
Also, a friend of mine suggested using QPixmap * instead of &. I tried it but never got it to work, the compiler was complaining and never liked the type QPixmap *.


Thanks a lot in advance for your help.

[faldzip]

What do you mean by not assigning local_pixmap anywhere? Isn't the emit emit clicked(local_pixmap) enough?

As you have only:


QPixmap local_pixmap;

it means your pixmap is blank. Are you setting any content to it?

[nmuntz]

As you have only:


QPixmap local_pixmap;

it means your pixmap is blank. Are you setting any content to it?

The content is being passed by my connect statement:


connect(labels[imagenum], SIGNAL(clicked(QPixmap&)), this, SLOT(setImage(QPixmap&)));


Which is supposed to copy the Pixmap from one QLabel into another.

[faldzip]

The content is being passed by my connect statement:


connect(labels[imagenum], SIGNAL(clicked(QPixmap&)), this, SLOT(setImage(QPixmap&)));


Which is supposed to copy the Pixmap from one QLabel into another.
Yes, but you are sending local_pixmap which is blank. Maybe more clear.

You have your ImageLabel. In there you have local_pixmap. Your local_pixmap isn't set anywhere, but constructed with the default QPixmap constructor. Then you are sending that blank pixmap with your signal clicked(QPixmap&) to the slot setImage(QPixmap&) which is setting received (blank) pixmap. So your code is working. It is doing exactly what you wrote in your source files.

[nmuntz]

Yes, but you are sending local_pixmap which is blank. Maybe more clear.

You have your ImageLabel. In there you have local_pixmap. Your local_pixmap isn't set anywhere, but constructed with the default QPixmap constructor. Then you are sending that blank pixmap with your signal clicked(QPixmap&) to the slot setImage(QPixmap&) which is setting received (blank) pixmap. So your code is working. It is doing exactly what you wrote in your source files.

Ahhh! That makes sense. You are right !
Now I'm not really sure how to set local_pixmap to be the pixmap sent by the QLabel.
Should that be part of my ImageLabel::slotClicked() function ? And would I pass that from my MainWindow into ImageLabel ?
Would something like this work?


ImageLabel *im = new ImageLabel;
im.localpixmap = labels[imagenum]->pixmap();

[faldzip]

hmm I can't really understand clearly your code. What's the labels array? As I understand from previous code fragments its something like this:


ImageLabel * labels[NUMBER];


Now, tell me, what pixmap you want to set where? If you want to have a bunch of ImageLabels with pixmap set on them you can do it like this:


for (int i = 0; i < NUMBER; ++i)
{
labels[i]->setPixmap(some_nice_pixmap);
}

And that labels will be shown in your widget with that pixmap.
If it's want you want to, you can send it like here:


emit clicked(*pixmap());

And you don't need any additional local_pixmap;

[nmuntz]

hmm I can't really understand clearly your code. What's the labels array? As I understand from previous code fragments its something like this:


ImageLabel * labels[NUMBER];


Now, tell me, what pixmap you want to set where? If you want to have a bunch of ImageLabels with pixmap set on them you can do it like this:


for (int i = 0; i < NUMBER; ++i)
{
labels[i]->setPixmap(some_nice_pixmap);
}

And that labels will be shown in your widget with that pixmap.
If it's want you want to, you can send it like here:


emit clicked(pixmap());

And you don't need any additional local_pixmap;



QList<ImageLabel *> labels;
int imagenum = 0;
foreach(const QString &path, files) {
QPixmap px(path);
labels.append(new ImageLabel);
labels[imagenum]->setPixmap(px.scaledToHeight(90));
qhbox->addWidget(labels[imagenum]);
connect(labels[imagenum], SIGNAL(clicked(const QPixmap&)), this, SLOT(setImage(const QPixmap &)));
imagenum++;
}


thats the relevant code. what it does is load all the images in a dir as thumnails and put them in a qhbox. and what I'm actually trying to do is that when I click on any of these ImageLabels it will load up the image on q mainimage label which is the full size image.

Thats why I subclassed QLabel so that it supports the clicked() event.

Thanks a lot for all your help!

[wysota]

You are overcomplicating things. If you want a clickable label, all you need to do is this:


class ClickableLabel : public QLabel {
Q_OBJECT
public:
ClickableLabel(QWidget *parent=0) : QLabel(parent){}
signals:
void clicked(const QPixmap &);
void clicked(const QString &);
protected:
void mouseReleaseEvent(QMouseEvent *e){
if(e->button()==Qt::LeftButton){
if(pixmap()) emit clicked(*pixmap()); // has a pixmap set
else if(!text.isEmpty()) emit clicked(text()); // has text set
else e->ignore(); // is empty, let's ignore the event, maybe the parent will handle it
} else e->ignore(); // wrong button released - ignore
}
};

You don't need to store the pixmap anywhere as QLabel already does that.

[nmuntz]

Wysota:

Thank worked perfectly !! Thank you very much!!!
Good call before... I was obviously doing it wrong and I overcomplicated things.

Thank you very much to both of you for all your help!

[faldzip]

Ahh! I forgot "*" :crying: ...