crash when calling new operator outside constructor in qt

I'm having a question about a weird (At least it was unexpected for me) behavior (It crashes) of qt when initializing pointers on a member class different than the constructor. I am attaching part of my code:

In mainwindow.h:

class MainWindow : public QMainWindow
{
 ...
 private:
     QPixmap *qpm_s1_yaw;
     QPainter *s1_yaw_painter;
     ...
}

In mainwindow.cpp:

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
 ...
 initGraph(qpm_s1_yaw, s1_yaw_painter, ui->YAW1);
 ...
}

void MainWindow::initGraph(QPixmap *map, QPainter *painter, QLabel *label)
{
map = new QPixmap(label->size());
map->fill(Qt::white);
painter = new QPainter(map);

... doing some stuff ...

label->setPixmap(*map); // ++(Remember this LINE)++
}

That actually works, but when I comment the line:

label->setPixmap(*map)

and instead set the Pixmap in the constructor (MainWindow::MainWindow) by writing:

ui->YAW1->setPixmap(*qpm_s1_yaw)

I got a segmentation Fault.

Could someone explain what is wrong with it? To make it work I had to initialize all the pointers in the constructor (and commenting those line in the classs member initGraph), like this:

qpm_s1_yaw = new QPixmap(ui->YAW1->size());
s1_yaw_painter = new QPainter(qpm_s1_yaw);
initGraph(qpm_s1_yaw, s1_yaw_painter, ui->YAW1);
ui->YAW1->setPixmap(*qpm_s1_yaw);

Thanks

Upvotes: 0

Views: 297

Answers (1)

This is a trivial misunderstanding of how C++ works, nothing to do with Qt.

Your code lies to you: you can equally well write: initGraph(0, 0, ui->YAW1). You're initializing local variables instead of class members. The values you pass as the first two arguments are not used for anything.

It's also completely unnecessary to hold the pixmap and the painter by pointer. Hold the pixmap by value, and only instantiate a painter for it when you do the painting.

Holding a painter to a pixmap when you're not painting on it can cause unnecessary copies of the pixmap to be made when the pixmap is consumed (read from): a pixmap with an active painter is considered "dirty".

What you should do then is to hold pixmaps by value and you can return the new value from initGraph - this decouples initGraph from the detail of the surrounding class where the pixmaps are stored. The user of initGraph has an option of not storing the pixmap, and e.g. querying the label itself for it.

class MainWindow : public QMainWindow
{
  Ui::MainWindow ui; // hold by value
  ...
  QPixmap qpm_s1_yaw; // hold by value
  QPixmap initGraph(QLabel *label) {
    QPixmap pixmap{label->size()};
    pixmap.fill(Qt::white);
    QPainter painter{&pixmap};
    //... doing some stuff ...
    label->setPixmap(pixmap);
    return pixmap;
  }
public:
  explicit MainWindow(QWidget *parent = nullptr) : QMainWindow(parent) {
    ui.setupUi(this);
    gpm_s1_yaw = initGraph(ui.YAW1);
  }
};

Upvotes: 1

Related Questions